Linux-GPIO Archive on lore.kernel.org
 help / color / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: Drew Fustini <drew@pdp7.com>
Cc: linux-gpio@vger.kernel.org, brgl@bgdev.pl,
	linus.walleij@linaro.org, thomas.petazzoni@bootlin.com
Subject: Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace
Date: Tue, 8 Oct 2019 14:15:12 +0800
Message-ID: <20191008061512.GA19956@sol> (raw)
In-Reply-To: <20190921102522.8970-1-drew@pdp7.com>

On Sat, Sep 21, 2019 at 12:25:23PM +0200, Drew Fustini wrote:
> Add pull-up/pull-down flags to the gpio line get and
> set ioctl() calls.  Use cases include a push button
> that does not have an external resistor.
> 
> Addition use cases described by Limor Fried (ladyada) of
> Adafruit in this PR for Adafruit_Blinka Python lib:
> https://github.com/adafruit/Adafruit_Blinka/pull/59
> 
> Signed-off-by: Drew Fustini <drew@pdp7.com>
> ---
>  drivers/gpio/gpiolib.c    | 12 ++++++++++++
>  include/uapi/linux/gpio.h |  4 ++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index d9074191edef..9da1093cc7f5 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -427,6 +427,8 @@ struct linehandle_state {
>  	(GPIOHANDLE_REQUEST_INPUT | \
>  	GPIOHANDLE_REQUEST_OUTPUT | \
>  	GPIOHANDLE_REQUEST_ACTIVE_LOW | \
> +	GPIOHANDLE_REQUEST_PULL_UP | \
> +	GPIOHANDLE_REQUEST_PULL_DOWN | \
>  	GPIOHANDLE_REQUEST_OPEN_DRAIN | \
>  	GPIOHANDLE_REQUEST_OPEN_SOURCE)
>  
> @@ -598,6 +600,10 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
>  			set_bit(FLAG_OPEN_DRAIN, &desc->flags);
>  		if (lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE)
>  			set_bit(FLAG_OPEN_SOURCE, &desc->flags);
> +		if (lflags & GPIOHANDLE_REQUEST_PULL_DOWN)
> +			set_bit(FLAG_PULL_DOWN, &desc->flags);
> +		if (lflags & GPIOHANDLE_REQUEST_PULL_UP)
> +			set_bit(FLAG_PULL_UP, &desc->flags);
>  
>  		ret = gpiod_set_transitory(desc, false);
>  		if (ret < 0)
> @@ -1102,6 +1108,10 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  		if (test_bit(FLAG_OPEN_SOURCE, &desc->flags))
>  			lineinfo.flags |= (GPIOLINE_FLAG_OPEN_SOURCE |
>  					   GPIOLINE_FLAG_IS_OUT);
> +		if (test_bit(FLAG_PULL_DOWN, &desc->flags))
> +			lineinfo.flags |= GPIOLINE_FLAG_PULL_DOWN;
> +		if (test_bit(FLAG_PULL_UP, &desc->flags))
> +			lineinfo.flags |= GPIOLINE_FLAG_PULL_UP;
>  
>  		if (copy_to_user(ip, &lineinfo, sizeof(lineinfo)))
>  			return -EFAULT;
> @@ -2475,6 +2485,8 @@ static bool gpiod_free_commit(struct gpio_desc *desc)
>  		clear_bit(FLAG_REQUESTED, &desc->flags);
>  		clear_bit(FLAG_OPEN_DRAIN, &desc->flags);
>  		clear_bit(FLAG_OPEN_SOURCE, &desc->flags);
> +		clear_bit(FLAG_PULL_UP, &desc->flags);
> +		clear_bit(FLAG_PULL_DOWN, &desc->flags);
>  		clear_bit(FLAG_IS_HOGGED, &desc->flags);
>  		ret = true;
>  	}
> diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
> index 4ebfe0ac6c5b..c2d1f7d908d6 100644
> --- a/include/uapi/linux/gpio.h
> +++ b/include/uapi/linux/gpio.h
> @@ -33,6 +33,8 @@ struct gpiochip_info {
>  #define GPIOLINE_FLAG_ACTIVE_LOW	(1UL << 2)
>  #define GPIOLINE_FLAG_OPEN_DRAIN	(1UL << 3)
>  #define GPIOLINE_FLAG_OPEN_SOURCE	(1UL << 4)
> +#define GPIOLINE_FLAG_PULL_UP	(1UL << 5)
> +#define GPIOLINE_FLAG_PULL_DOWN	(1UL << 6)
>  
>  /**
>   * struct gpioline_info - Information about a certain GPIO line
> @@ -62,6 +64,8 @@ struct gpioline_info {
>  #define GPIOHANDLE_REQUEST_ACTIVE_LOW	(1UL << 2)
>  #define GPIOHANDLE_REQUEST_OPEN_DRAIN	(1UL << 3)
>  #define GPIOHANDLE_REQUEST_OPEN_SOURCE	(1UL << 4)
> +#define GPIOHANDLE_REQUEST_PULL_UP	(1UL << 5)
> +#define GPIOHANDLE_REQUEST_PULL_DOWN	(1UL << 6)
>  
>  /**
>   * struct gpiohandle_request - Information about a GPIO handle request
> -- 
> 2.20.1
> 
Sorry for the late feedback, but I'm still not sure whether this patch
is on track to be applied or not.  I had thought not, at least not yet,
but just in case...

You have added the flags to linehandle_create, but not lineevent_create.
Given that your primary use case is push buttons, isn't the event request
at least as important as the handle request?  Even ignoring your use
case, I'd add them to lineevent_create just to maintain symmetry.

Also, is it valid to set PULL_UP and PULL_DOWN simulaneously?
I would think not, but I could be wrong.
If not then that combination should be rejected with EINVAL.

Cheers,
Kent.

  parent reply index

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-21 10:25 Drew Fustini
2019-09-23  8:38 ` Bartosz Golaszewski
2019-10-02 11:10   ` Drew Fustini
2019-10-03 12:47   ` Linus Walleij
2019-10-04  7:22     ` Bartosz Golaszewski
2019-10-04 12:46       ` Bartosz Golaszewski
2019-10-05 17:02       ` Linus Walleij
2019-10-06  3:12         ` Kent Gibson
2019-10-06 21:06           ` Linus Walleij
2019-10-07  4:37             ` Kent Gibson
2019-10-08  6:15 ` Kent Gibson [this message]
2019-10-08 20:56   ` Bartosz Golaszewski
2019-10-08 23:21     ` Kent Gibson
2019-10-08 23:30       ` Bartosz Golaszewski
2019-10-08 23:56         ` Kent Gibson
2019-10-09  0:03           ` Bartosz Golaszewski
2019-10-09  0:22             ` Kent Gibson
2019-10-09  6:55               ` Kent Gibson
2019-10-09 12:57                 ` Drew Fustini
2019-10-09 13:23                   ` Kent Gibson
2019-10-09 13:30                 ` Drew Fustini
2019-10-09 14:11                   ` Kent Gibson
2019-10-09 15:50                     ` Bartosz Golaszewski
2019-10-09 16:19                       ` Kent Gibson
2019-10-09 23:59                   ` Kent Gibson
2019-10-10  7:47                     ` Drew Fustini
2019-10-10 10:14                       ` Kent Gibson
2019-10-10 11:17                         ` Kent Gibson
2019-10-11 13:04                           ` Drew Fustini
2019-10-11 13:06                         ` Drew Fustini
2019-10-11 13:49                           ` Kent Gibson
2019-10-09 11:32               ` Drew Fustini
2019-10-09 13:57       ` Drew Fustini
2019-10-09 14:01         ` Kent Gibson
2019-10-09 11:46   ` Drew Fustini

Reply instructions:

You may reply publically 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=20191008061512.GA19956@sol \
    --to=warthog618@gmail.com \
    --cc=brgl@bgdev.pl \
    --cc=drew@pdp7.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=thomas.petazzoni@bootlin.com \
    /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

Linux-GPIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-gpio/0 linux-gpio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-gpio linux-gpio/ https://lore.kernel.org/linux-gpio \
		linux-gpio@vger.kernel.org
	public-inbox-index linux-gpio

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-gpio


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git