[RFC] gpio: expose pull-up/pull-down line flags to userspace

[RFC] gpio: expose pull-up/pull-down line flags to userspace

[Drew Fustini]

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

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Bartosz Golaszewski]

sob., 21 wrz 2019 o 12:27 Drew Fustini <drew@pdp7.com> napisał(a):
>
> 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
>

Hi Drew,

I remember discussing it with Linus some time ago. This may not be as
straightforward as simply adding new flags. Since PULL-UP/DOWN
resistors can - among other parameters - also have configurable
resistance, we'll probably need some kind of a structure for this
ioctl() to pass any additional information to the kernel. Since we
can't change ABI this may require adding a whole new ioctl() for
extended configuration. This in turn has to be as future-proof as
possible - if someone asks for user-space-configurable drive-strength,
the new ioctl() should be ready for it.

I should have some bandwidth in the coming days, so I'll try to give it a try.

Bart

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Drew Fustini]

On Mon, Sep 23, 2019 at 10:38 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> I remember discussing it with Linus some time ago. This may not be as
> straightforward as simply adding new flags. Since PULL-UP/DOWN
> resistors can - among other parameters - also have configurable
> resistance, we'll probably need some kind of a structure for this
> ioctl() to pass any additional information to the kernel. Since we
> can't change ABI this may require adding a whole new ioctl() for
> extended configuration. This in turn has to be as future-proof as
> possible - if someone asks for user-space-configurable drive-strength,
> the new ioctl() should be ready for it.

Thanks for the insight, Bartosz.  Would you be able to point me to the
discussion if is available in an archive?  I would like to improve my
understanding.

What do you think of adding pull-up/down flags first, and then
implementing the extended configuration ioctl() later if necessary?

Thank you,
Drew

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Linus Walleij]

On Mon, Sep 23, 2019 at 10:38 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> I remember discussing it with Linus some time ago. This may not be as
> straightforward as simply adding new flags. Since PULL-UP/DOWN
> resistors can - among other parameters - also have configurable
> resistance, we'll probably need some kind of a structure for this
> ioctl() to pass any additional information to the kernel. Since we
> can't change ABI this may require adding a whole new ioctl() for
> extended configuration. This in turn has to be as future-proof as
> possible - if someone asks for user-space-configurable drive-strength,
> the new ioctl() should be ready for it.
>
> I should have some bandwidth in the coming days, so I'll try to give it a try.

What we did for the in-kernel API and the Device Tree ABI
was to simply say that if you need such elaborate control over
the line, it needs to be done with a proper pin control driver.

So for lines that just have the GPIO_PULL_UP or
GPIO_PULL_DOWN set as a (one-bit) flag, what you will
get is "typical" pull down/up (whatever the hardware default
is, or what the driver thinks is default, which should be safe
so the highest possible pull resistance I suppose).

So one option is to just go with these flags and explicitly
say that it will give a "system default (high resistance)
pull up/down".

That said, the pin controller back-end is fully capable of
accepting more elaborate configuration, so if we prefer then
we can make the more complex userspace ABI that can
set it to a desired-or-default resistance.

I lean toward simplicity here. I haven't seen that these
userspace consumers need very elaborate control of this
resistance, they are for one-off hacks and as such should
be fine with just default pull up/down I think?

I  think that specifying "this line will use pull up/down"
at request time and having the driver set a safe default
pull-up/down as response, (and pretty much what this
patch does) and then add another explicit
ioctl to refine it the day we need it is a viable way forward.

 in the future something like:
#define GPIOHANDLE_SET_LINE_CONFIG_IOCTL _IOWR(0xB4, 0x0a, struct
gpiohandle_config)

And then, when we need it, try to come up with some
really flexible ABI for the config, based on
include/linux/pinctrl/pinconf-generic.h

But no upfront code for that right now as it is not needed.
A practical usecase must come first.

Yours,
Linus Walleij

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Bartosz Golaszewski]

czw., 3 paź 2019 o 14:47 Linus Walleij <linus.walleij@linaro.org> napisał(a):
>
> On Mon, Sep 23, 2019 at 10:38 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > I remember discussing it with Linus some time ago. This may not be as
> > straightforward as simply adding new flags. Since PULL-UP/DOWN
> > resistors can - among other parameters - also have configurable
> > resistance, we'll probably need some kind of a structure for this
> > ioctl() to pass any additional information to the kernel. Since we
> > can't change ABI this may require adding a whole new ioctl() for
> > extended configuration. This in turn has to be as future-proof as
> > possible - if someone asks for user-space-configurable drive-strength,
> > the new ioctl() should be ready for it.
> >
> > I should have some bandwidth in the coming days, so I'll try to give it a try.
>
> What we did for the in-kernel API and the Device Tree ABI
> was to simply say that if you need such elaborate control over
> the line, it needs to be done with a proper pin control driver.
>
> So for lines that just have the GPIO_PULL_UP or
> GPIO_PULL_DOWN set as a (one-bit) flag, what you will
> get is "typical" pull down/up (whatever the hardware default
> is, or what the driver thinks is default, which should be safe
> so the highest possible pull resistance I suppose).
>
> So one option is to just go with these flags and explicitly
> say that it will give a "system default (high resistance)
> pull up/down".
>
> That said, the pin controller back-end is fully capable of
> accepting more elaborate configuration, so if we prefer then
> we can make the more complex userspace ABI that can
> set it to a desired-or-default resistance.
>
> I lean toward simplicity here. I haven't seen that these
> userspace consumers need very elaborate control of this
> resistance, they are for one-off hacks and as such should
> be fine with just default pull up/down I think?
>
> I  think that specifying "this line will use pull up/down"
> at request time and having the driver set a safe default
> pull-up/down as response, (and pretty much what this
> patch does) and then add another explicit
> ioctl to refine it the day we need it is a viable way forward.
>
>  in the future something like:
> #define GPIOHANDLE_SET_LINE_CONFIG_IOCTL _IOWR(0xB4, 0x0a, struct
> gpiohandle_config)
>
> And then, when we need it, try to come up with some
> really flexible ABI for the config, based on
> include/linux/pinctrl/pinconf-generic.h
>

Thanks for your input Linus. I'm good with that. The config ioctl (or
something similar) you're mentioning may appear sooner actually -
users of libgpiod have been requesting a way of changing the direction
of a line without releasing it - something that's possible in the
kernel, but not from user-space at the moment. I'll submit something
that allows to change the configuration of a requested line soon.

Bart

> But no upfront code for that right now as it is not needed.
> A practical usecase must come first.
>
> Yours,
> Linus Walleij

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Bartosz Golaszewski]

pt., 4 paź 2019 o 09:22 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
>
> czw., 3 paź 2019 o 14:47 Linus Walleij <linus.walleij@linaro.org> napisał(a):
> >
> > On Mon, Sep 23, 2019 at 10:38 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > > I remember discussing it with Linus some time ago. This may not be as
> > > straightforward as simply adding new flags. Since PULL-UP/DOWN
> > > resistors can - among other parameters - also have configurable
> > > resistance, we'll probably need some kind of a structure for this
> > > ioctl() to pass any additional information to the kernel. Since we
> > > can't change ABI this may require adding a whole new ioctl() for
> > > extended configuration. This in turn has to be as future-proof as
> > > possible - if someone asks for user-space-configurable drive-strength,
> > > the new ioctl() should be ready for it.
> > >
> > > I should have some bandwidth in the coming days, so I'll try to give it a try.
> >
> > What we did for the in-kernel API and the Device Tree ABI
> > was to simply say that if you need such elaborate control over
> > the line, it needs to be done with a proper pin control driver.
> >
> > So for lines that just have the GPIO_PULL_UP or
> > GPIO_PULL_DOWN set as a (one-bit) flag, what you will
> > get is "typical" pull down/up (whatever the hardware default
> > is, or what the driver thinks is default, which should be safe
> > so the highest possible pull resistance I suppose).
> >
> > So one option is to just go with these flags and explicitly
> > say that it will give a "system default (high resistance)
> > pull up/down".
> >
> > That said, the pin controller back-end is fully capable of
> > accepting more elaborate configuration, so if we prefer then
> > we can make the more complex userspace ABI that can
> > set it to a desired-or-default resistance.
> >
> > I lean toward simplicity here. I haven't seen that these
> > userspace consumers need very elaborate control of this
> > resistance, they are for one-off hacks and as such should
> > be fine with just default pull up/down I think?
> >
> > I  think that specifying "this line will use pull up/down"
> > at request time and having the driver set a safe default
> > pull-up/down as response, (and pretty much what this
> > patch does) and then add another explicit
> > ioctl to refine it the day we need it is a viable way forward.
> >
> >  in the future something like:
> > #define GPIOHANDLE_SET_LINE_CONFIG_IOCTL _IOWR(0xB4, 0x0a, struct
> > gpiohandle_config)
> >
> > And then, when we need it, try to come up with some
> > really flexible ABI for the config, based on
> > include/linux/pinctrl/pinconf-generic.h
> >
>
> Thanks for your input Linus. I'm good with that. The config ioctl (or
> something similar) you're mentioning may appear sooner actually -
> users of libgpiod have been requesting a way of changing the direction
> of a line without releasing it - something that's possible in the
> kernel, but not from user-space at the moment. I'll submit something
> that allows to change the configuration of a requested line soon.
>
> Bart

Cc'ing Kent, as he might be interested in this.

I'll be travelling tomorrow and will have a couple hours on the plane,
so I'll try to cook up an additional ioctl for configuration changes
of requested lines with additional padding for future extension.

Bart

>
> > But no upfront code for that right now as it is not needed.
> > A practical usecase must come first.
> >
> > Yours,
> > Linus Walleij

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Linus Walleij]

On Fri, Oct 4, 2019 at 9:22 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> The config ioctl (or
> something similar) you're mentioning may appear sooner actually -
> users of libgpiod have been requesting a way of changing the direction
> of a line without releasing it - something that's possible in the
> kernel, but not from user-space at the moment. I'll submit something
> that allows to change the configuration of a requested line soon.

Hm! I guess I assumed that userspace users would be using the lines
for either input or output, not complex use cases like that, reversing
direction and what not.

What kind of usecase is this? I certainly hope nothing like doing
userspace drivers for complex hardware ... those should be in
the kernel... the current ABI is a bit oriented around industrial
automation and prototyping use cases.

Yours,
Linus Walleij

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Kent Gibson]

On Sat, Oct 05, 2019 at 07:02:58PM +0200, Linus Walleij wrote:
> On Fri, Oct 4, 2019 at 9:22 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> 
> > The config ioctl (or
> > something similar) you're mentioning may appear sooner actually -
> > users of libgpiod have been requesting a way of changing the direction
> > of a line without releasing it - something that's possible in the
> > kernel, but not from user-space at the moment. I'll submit something
> > that allows to change the configuration of a requested line soon.
> 
> Hm! I guess I assumed that userspace users would be using the lines
> for either input or output, not complex use cases like that, reversing
> direction and what not.
> 
> What kind of usecase is this? I certainly hope nothing like doing
> userspace drivers for complex hardware ... those should be in
> the kernel... the current ABI is a bit oriented around industrial
> automation and prototyping use cases.
> 
I'm not the only one asking for this, and I can't speak to others' use
cases, but in my case I'm prototyping and bit bashing a SPI driver for
an ADC where the MOSI/MISO lines can be tied to save a pin. I need to
be able to switch line direction without glitching the line and that
can't be guaranteed with the current UAPI.

Would you consider that "complex hardware"?

Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Linus Walleij]

On Sun, Oct 6, 2019 at 5:12 AM Kent Gibson <warthog618@gmail.com> wrote:
> On Sat, Oct 05, 2019 at 07:02:58PM +0200, Linus Walleij wrote:
> > On Fri, Oct 4, 2019 at 9:22 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > > The config ioctl (or
> > > something similar) you're mentioning may appear sooner actually -
> > > users of libgpiod have been requesting a way of changing the direction
> > > of a line without releasing it - something that's possible in the
> > > kernel, but not from user-space at the moment. I'll submit something
> > > that allows to change the configuration of a requested line soon.
> >
> > Hm! I guess I assumed that userspace users would be using the lines
> > for either input or output, not complex use cases like that, reversing
> > direction and what not.
> >
> > What kind of usecase is this? I certainly hope nothing like doing
> > userspace drivers for complex hardware ... those should be in
> > the kernel... the current ABI is a bit oriented around industrial
> > automation and prototyping use cases.
>
> I'm not the only one asking for this, and I can't speak to others' use
> cases, but in my case I'm prototyping and bit bashing a SPI driver for
> an ADC where the MOSI/MISO lines can be tied to save a pin. I need to
> be able to switch line direction without glitching the line and that
> can't be guaranteed with the current UAPI.
>
> Would you consider that "complex hardware"?

I see, so in the Linux kernel this is called 3-wire SPI, and we would
drive the SPI bit-banged bus using drivers/spi/spi-gpio.c and
model ADC "foo" it as an ADC in drivers/iio/adc/foo.c.

The device tree would look something like so:

spi {
    compatible = "spi-gpio";
    #address-cells = <1>;
    #size-cells = <0>;

    gpio-sck = <&gpio0 5 GPIO_ACTIVE_HIGH>;
    gpio-mosi = <&gpio0 8 GPIO_ACTIVE_HIGH>;
    cs-gpios = <&gpio0 20 GPIO_ACTIVE_LOW>;
    num-chipselects = <1>;

    adc {
        compatible = "foo";
        reg = <0>;
        spi-3wire;
    };
};

(There are ways to define devices without explicit chipselect
as well, yielding a 2-wire bus in the end.)

The goal of the bit-banged SPI GPIO driver is to have one
driver that can be reused for any GPIO bit-banged SPI.

The goal of IIO ADC is to present a uniform and scaled
ADC interface to userspace, such that userspace need not
worry which ADC it is using because they all present the
same user API. IIO further provides precise triggers
using timers or interrupt generators to provide a
continous stream of AD-converted values using a
ring buffer.

So in summary the kernels SPI, GPIO and IIO subsystems
want to abstract hardware and make userspace simpler,
and the kernel sees userspace as having no business
driving hardware.

This might be seen as a bit imperialistic, but alas, the
explicit goal of kernel development is hardware abstraction,
and what happens sometimes in the maker community
in particular is to reimplement what the kernel does in
userspace.

What we are especially worried about is that for systems
that are not one-offs and lab prototypes, something that is
getting productified, the userspace tinkering and hackering
is just preserved in resin and no transition to proper Linux
drivers happens.

So we want userspace GPIO access to be an enabler and
not a competitor to the Linux kernel.

There are other mechanisms in the kernel with this problem,
such as UIO.

There is a problem with the biases here, such that some of the
kernel people always think they are dealing with companies
and that kind of larger organizations, and I am one of those
that don't think like that.

When it does come to companies, I think
there is this quote by Fred Brooks that you should
"always plan to throw one away" so they might write a
userspace prototype, then
throw that away and write a kernel driver as one gained
knowledge of how the hardware works. It's one of the ways
to spin it. And we want to encourage that way of doing
some userspace prototyping. And if the usecase gets
complex enough it will lean toward: what about just writing
a kernel driver in the first place and refine that.

If we are thinking a hobbyist, they are thinking to drive this
thing on their table for the next two hours or as an experiment
or for fun, or semi business (a few devices, not really mass
production), like to set up a factory line or a lightshow, that
is something that should stay in userspace and has no business
in the kernel.

And we just need to know what usecase it is here. But I need
to understand.

Yours,
Linus Walleij

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Kent Gibson]

On Sun, Oct 06, 2019 at 11:06:21PM +0200, Linus Walleij wrote:
> On Sun, Oct 6, 2019 at 5:12 AM Kent Gibson <warthog618@gmail.com> wrote:
> > On Sat, Oct 05, 2019 at 07:02:58PM +0200, Linus Walleij wrote:
> > > On Fri, Oct 4, 2019 at 9:22 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > >
> > > > The config ioctl (or
> > > > something similar) you're mentioning may appear sooner actually -
> > > > users of libgpiod have been requesting a way of changing the direction
> > > > of a line without releasing it - something that's possible in the
> > > > kernel, but not from user-space at the moment. I'll submit something
> > > > that allows to change the configuration of a requested line soon.
> > >
> > > Hm! I guess I assumed that userspace users would be using the lines
> > > for either input or output, not complex use cases like that, reversing
> > > direction and what not.
> > >
> > > What kind of usecase is this? I certainly hope nothing like doing
> > > userspace drivers for complex hardware ... those should be in
> > > the kernel... the current ABI is a bit oriented around industrial
> > > automation and prototyping use cases.
> >
> > I'm not the only one asking for this, and I can't speak to others' use
> > cases, but in my case I'm prototyping and bit bashing a SPI driver for
> > an ADC where the MOSI/MISO lines can be tied to save a pin. I need to
> > be able to switch line direction without glitching the line and that
> > can't be guaranteed with the current UAPI.
> >
> > Would you consider that "complex hardware"?
> 
> I see, so in the Linux kernel this is called 3-wire SPI, and we would
> drive the SPI bit-banged bus using drivers/spi/spi-gpio.c and
> model ADC "foo" it as an ADC in drivers/iio/adc/foo.c.
> 
> The device tree would look something like so:
> 
> spi {
>     compatible = "spi-gpio";
>     #address-cells = <1>;
>     #size-cells = <0>;
> 
>     gpio-sck = <&gpio0 5 GPIO_ACTIVE_HIGH>;
>     gpio-mosi = <&gpio0 8 GPIO_ACTIVE_HIGH>;
>     cs-gpios = <&gpio0 20 GPIO_ACTIVE_LOW>;
>     num-chipselects = <1>;
> 
>     adc {
>         compatible = "foo";
>         reg = <0>;
>         spi-3wire;
>     };
> };
> 
> (There are ways to define devices without explicit chipselect
> as well, yielding a 2-wire bus in the end.)
> 
> The goal of the bit-banged SPI GPIO driver is to have one
> driver that can be reused for any GPIO bit-banged SPI.
> 
> The goal of IIO ADC is to present a uniform and scaled
> ADC interface to userspace, such that userspace need not
> worry which ADC it is using because they all present the
> same user API. IIO further provides precise triggers
> using timers or interrupt generators to provide a
> continous stream of AD-converted values using a
> ring buffer.
> 
> So in summary the kernels SPI, GPIO and IIO subsystems
> want to abstract hardware and make userspace simpler,
> and the kernel sees userspace as having no business
> driving hardware.
> 
> This might be seen as a bit imperialistic, but alas, the
> explicit goal of kernel development is hardware abstraction,
> and what happens sometimes in the maker community
> in particular is to reimplement what the kernel does in
> userspace.
> 
> What we are especially worried about is that for systems
> that are not one-offs and lab prototypes, something that is
> getting productified, the userspace tinkering and hackering
> is just preserved in resin and no transition to proper Linux
> drivers happens.
> 
> So we want userspace GPIO access to be an enabler and
> not a competitor to the Linux kernel.
> 
> There are other mechanisms in the kernel with this problem,
> such as UIO.
> 
> There is a problem with the biases here, such that some of the
> kernel people always think they are dealing with companies
> and that kind of larger organizations, and I am one of those
> that don't think like that.
> 
> When it does come to companies, I think
> there is this quote by Fred Brooks that you should
> "always plan to throw one away" so they might write a
> userspace prototype, then
> throw that away and write a kernel driver as one gained
> knowledge of how the hardware works. It's one of the ways
> to spin it. And we want to encourage that way of doing
> some userspace prototyping. And if the usecase gets
> complex enough it will lean toward: what about just writing
> a kernel driver in the first place and refine that.
> 
> If we are thinking a hobbyist, they are thinking to drive this
> thing on their table for the next two hours or as an experiment
> or for fun, or semi business (a few devices, not really mass
> production), like to set up a factory line or a lightshow, that
> is something that should stay in userspace and has no business
> in the kernel.
> 
> And we just need to know what usecase it is here. But I need
> to understand.
> 
Most definitely thinking hobbyist applications, so stepping up to 
DT/SPI/IIO would be massive overkill.

While I understand the desire that those kernel features be used where
appropriate, there are still cases where just being able to drive a
GPIO pin from userspace is the best solution.

I've been working with the Raspberry Pi where the accepted approach
to userspace GPIO is based on gpiomem - a kernel module that provides
memory mapped access to the bcm2835 registers.  For reasons that I hope
are obvious, I would like to move away from that to standard kernel
APIs, so the GPIO UAPI.

The changes to the GPIO UAPI that I've been discussing with Bart,
(the direction change and pull up/down support) are to cover 
reasonably common operations that currently only have kernel
space solutions.

I'm happy to implement those changes, btw, but want to ensure
that I'm on the right track so I don't go wasting anyone's time.

Cheers,
Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Kent Gibson]

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.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Bartosz Golaszewski]

wt., 8 paź 2019 o 08:15 Kent Gibson <warthog618@gmail.com> napisał(a):
>
> 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...
>

It still needs some fixes, but Linus seems to be fine with the general idea.

> 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.
>

Yes, some validity checks of the flags must be added. I even did it in
my local branch[1].

Re: direction and configuration changes on requested lines: I think
it's quite useful to add in the form of a new ioctl():
GPIOHANDLE_SET_CONFIG_IOCTL or something. I started hacking on this
but eventually got more busy this week than I anticipated. I'm still
planning on sending an RFC by the end of the week though.

Bart

[1] https://github.com/brgl/linux/commit/82fc38f6ab599ee03f7a8ed078de8abb41e6e611

> Cheers,
> Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Kent Gibson]

On Tue, Oct 08, 2019 at 10:56:18PM +0200, Bartosz Golaszewski wrote:
> wt., 8 paź 2019 o 08:15 Kent Gibson <warthog618@gmail.com> napisał(a):
> >
> > 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...
> >
> 
> It still needs some fixes, but Linus seems to be fine with the general idea.
> 
> > 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.
> >
> 
> Yes, some validity checks of the flags must be added. I even did it in
> my local branch[1].
>
Your changes for linehandle_create look ok, but for lineevent_create
you explicitly disabled PULL_UP and PULL_DOWN, and in a block labelled
"This is just wrong: we don't look for events on output lines" at that.

> Re: direction and configuration changes on requested lines: I think
> it's quite useful to add in the form of a new ioctl():
> GPIOHANDLE_SET_CONFIG_IOCTL or something. I started hacking on this
> but eventually got more busy this week than I anticipated. I'm still
> planning on sending an RFC by the end of the week though.
>
I have the reverse problem - bored and looking for something to do, so
more than willing to help out - if you want it.

And while we're talking, does the gpio-mockup support pull up/down
being set from the kernel side?

Cheers,
Kent.
> 
> [1] https://github.com/brgl/linux/commit/82fc38f6ab599ee03f7a8ed078de8abb41e6e611
> 
> > Cheers,
> > Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Bartosz Golaszewski]

śr., 9 paź 2019 o 01:21 Kent Gibson <warthog618@gmail.com> napisał(a):
>
> On Tue, Oct 08, 2019 at 10:56:18PM +0200, Bartosz Golaszewski wrote:
> > wt., 8 paź 2019 o 08:15 Kent Gibson <warthog618@gmail.com> napisał(a):
> > >
> > > 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...
> > >
> >
> > It still needs some fixes, but Linus seems to be fine with the general idea.
> >
> > > 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.
> > >
> >
> > Yes, some validity checks of the flags must be added. I even did it in
> > my local branch[1].
> >
> Your changes for linehandle_create look ok, but for lineevent_create
> you explicitly disabled PULL_UP and PULL_DOWN, and in a block labelled
> "This is just wrong: we don't look for events on output lines" at that.
>

Yeah, feel free to change it.

> > Re: direction and configuration changes on requested lines: I think
> > it's quite useful to add in the form of a new ioctl():
> > GPIOHANDLE_SET_CONFIG_IOCTL or something. I started hacking on this
> > but eventually got more busy this week than I anticipated. I'm still
> > planning on sending an RFC by the end of the week though.
> >
> I have the reverse problem - bored and looking for something to do, so
> more than willing to help out - if you want it.
>

Sure, in that case let's just wait for your patches. You can just
extend what I started, if you wish.

> And while we're talking, does the gpio-mockup support pull up/down
> being set from the kernel side?
>

No, but feel free to add it.

Bart

> Cheers,
> Kent.
> >
> > [1] https://github.com/brgl/linux/commit/82fc38f6ab599ee03f7a8ed078de8abb41e6e611
> >
> > > Cheers,
> > > Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Kent Gibson]

On Wed, Oct 09, 2019 at 01:30:18AM +0200, Bartosz Golaszewski wrote:
> śr., 9 paź 2019 o 01:21 Kent Gibson <warthog618@gmail.com> napisał(a):
> >
> > On Tue, Oct 08, 2019 at 10:56:18PM +0200, Bartosz Golaszewski wrote:
> > > wt., 8 paź 2019 o 08:15 Kent Gibson <warthog618@gmail.com> napisał(a):
> > > >
> > > > 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...
> > > >
> > >
> > > It still needs some fixes, but Linus seems to be fine with the general idea.
> > >
> > > > 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.
> > > >
> > >
> > > Yes, some validity checks of the flags must be added. I even did it in
> > > my local branch[1].
> > >
> > Your changes for linehandle_create look ok, but for lineevent_create
> > you explicitly disabled PULL_UP and PULL_DOWN, and in a block labelled
> > "This is just wrong: we don't look for events on output lines" at that.
> >
> 
> Yeah, feel free to change it.
> 
Yeah, you see I didn't feel free to change it, as I thought the
appropriate etiquette was to send comments to the original author.
At least until the patch has been applied.
Or are RFCs a special case?

> > > Re: direction and configuration changes on requested lines: I think
> > > it's quite useful to add in the form of a new ioctl():
> > > GPIOHANDLE_SET_CONFIG_IOCTL or something. I started hacking on this
> > > but eventually got more busy this week than I anticipated. I'm still
> > > planning on sending an RFC by the end of the week though.
> > >
> > I have the reverse problem - bored and looking for something to do, so
> > more than willing to help out - if you want it.
> >
> 
> Sure, in that case let's just wait for your patches. You can just
> extend what I started, if you wish.
>
Yep, you sound tired.
Are we talking about the SET_CONFIG here?  If you have a link to what
you've started I'll take a look, but as I've already said - I'm not
about to go writing new ioctls without approval and guidance from you
and Linus.  If you have the ioctl defined I'm happy to take it from
there. Well happy-ish.

> > And while we're talking, does the gpio-mockup support pull up/down
> > being set from the kernel side?
> >
> 
> No, but feel free to add it.
>
Ok, will have a look - cos I would like to add tests for the above and
that will require the mockup supporting it as well.

Cheers,
Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Bartosz Golaszewski]

śr., 9 paź 2019 o 01:56 Kent Gibson <warthog618@gmail.com> napisał(a):
>
> On Wed, Oct 09, 2019 at 01:30:18AM +0200, Bartosz Golaszewski wrote:
> > śr., 9 paź 2019 o 01:21 Kent Gibson <warthog618@gmail.com> napisał(a):
> > >
> > > On Tue, Oct 08, 2019 at 10:56:18PM +0200, Bartosz Golaszewski wrote:
> > > > wt., 8 paź 2019 o 08:15 Kent Gibson <warthog618@gmail.com> napisał(a):
> > > > >
> > > > > 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...
> > > > >
> > > >
> > > > It still needs some fixes, but Linus seems to be fine with the general idea.
> > > >
> > > > > 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.
> > > > >
> > > >
> > > > Yes, some validity checks of the flags must be added. I even did it in
> > > > my local branch[1].
> > > >
> > > Your changes for linehandle_create look ok, but for lineevent_create
> > > you explicitly disabled PULL_UP and PULL_DOWN, and in a block labelled
> > > "This is just wrong: we don't look for events on output lines" at that.
> > >
> >
> > Yeah, feel free to change it.
> >
> Yeah, you see I didn't feel free to change it, as I thought the
> appropriate etiquette was to send comments to the original author.
> At least until the patch has been applied.
> Or are RFCs a special case?
>

So either you review it and the author changes it and resends, or - if
for example you want to build on top of someone else's changes - you
take the patch, keep the original authorship, modify it and add your
signed-off-by tag with a small note on your changes in square
brackets.

> > > > Re: direction and configuration changes on requested lines: I think
> > > > it's quite useful to add in the form of a new ioctl():
> > > > GPIOHANDLE_SET_CONFIG_IOCTL or something. I started hacking on this
> > > > but eventually got more busy this week than I anticipated. I'm still
> > > > planning on sending an RFC by the end of the week though.
> > > >
> > > I have the reverse problem - bored and looking for something to do, so
> > > more than willing to help out - if you want it.
> > >
> >
> > Sure, in that case let's just wait for your patches. You can just
> > extend what I started, if you wish.
> >
> Yep, you sound tired.
> Are we talking about the SET_CONFIG here?  If you have a link to what
> you've started I'll take a look, but as I've already said - I'm not
> about to go writing new ioctls without approval and guidance from you
> and Linus.  If you have the ioctl defined I'm happy to take it from
> there. Well happy-ish.
>

But there's absolutely nothing wrong with writing code even if you're
not sure about the solution - it's even encouraged[1]. It's much
easier to discuss existing code than potential solutions.

[1] https://lkml.org/lkml/2000/8/25/132

> > > And while we're talking, does the gpio-mockup support pull up/down
> > > being set from the kernel side?
> > >
> >
> > No, but feel free to add it.
> >
> Ok, will have a look - cos I would like to add tests for the above and
> that will require the mockup supporting it as wel

Do you also plan on extending libgpiod by any chance?

Bart

>
> Cheers,
> Kent.
>

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Kent Gibson]

On Wed, Oct 09, 2019 at 02:03:58AM +0200, Bartosz Golaszewski wrote:
> śr., 9 paź 2019 o 01:56 Kent Gibson <warthog618@gmail.com> napisał(a):
> >
> > On Wed, Oct 09, 2019 at 01:30:18AM +0200, Bartosz Golaszewski wrote:
> > > śr., 9 paź 2019 o 01:21 Kent Gibson <warthog618@gmail.com> napisał(a):
> > > >
> > > > On Tue, Oct 08, 2019 at 10:56:18PM +0200, Bartosz Golaszewski wrote:
> > > > > wt., 8 paź 2019 o 08:15 Kent Gibson <warthog618@gmail.com> napisał(a):
> > > > > >
> > > > > > 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...
> > > > > >
> > > > >
> > > > > It still needs some fixes, but Linus seems to be fine with the general idea.
> > > > >
> > > > > > 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.
> > > > > >
> > > > >
> > > > > Yes, some validity checks of the flags must be added. I even did it in
> > > > > my local branch[1].
> > > > >
> > > > Your changes for linehandle_create look ok, but for lineevent_create
> > > > you explicitly disabled PULL_UP and PULL_DOWN, and in a block labelled
> > > > "This is just wrong: we don't look for events on output lines" at that.
> > > >
> > >
> > > Yeah, feel free to change it.
> > >
> > Yeah, you see I didn't feel free to change it, as I thought the
> > appropriate etiquette was to send comments to the original author.
> > At least until the patch has been applied.
> > Or are RFCs a special case?
> >
> 
> So either you review it and the author changes it and resends, or - if
> for example you want to build on top of someone else's changes - you
> take the patch, keep the original authorship, modify it and add your
> signed-off-by tag with a small note on your changes in square
> brackets.
> 
Ok, but I'd still expect some feedback to the author to that effect.
Maybe I missed it.

> > > > > Re: direction and configuration changes on requested lines: I think
> > > > > it's quite useful to add in the form of a new ioctl():
> > > > > GPIOHANDLE_SET_CONFIG_IOCTL or something. I started hacking on this
> > > > > but eventually got more busy this week than I anticipated. I'm still
> > > > > planning on sending an RFC by the end of the week though.
> > > > >
> > > > I have the reverse problem - bored and looking for something to do, so
> > > > more than willing to help out - if you want it.
> > > >
> > >
> > > Sure, in that case let's just wait for your patches. You can just
> > > extend what I started, if you wish.
> > >
> > Yep, you sound tired.
> > Are we talking about the SET_CONFIG here?  If you have a link to what
> > you've started I'll take a look, but as I've already said - I'm not
> > about to go writing new ioctls without approval and guidance from you
> > and Linus.  If you have the ioctl defined I'm happy to take it from
> > there. Well happy-ish.
> >
> 
> But there's absolutely nothing wrong with writing code even if you're
> not sure about the solution - it's even encouraged[1]. It's much
> easier to discuss existing code than potential solutions.
> 
> [1] https://lkml.org/lkml/2000/8/25/132
> 
Exactly - show me your code.

> > > > And while we're talking, does the gpio-mockup support pull up/down
> > > > being set from the kernel side?
> > > >
> > >
> > > No, but feel free to add it.
> > >
> > Ok, will have a look - cos I would like to add tests for the above and
> > that will require the mockup supporting it as wel
> 
> Do you also plan on extending libgpiod by any chance?
> 
Well I thought that was your baby and was going to extend my Go library,
and leave libgpiod to you.  But I can help out with libgpiod as well.
I suppose you'll want the Python and C++ bindings updated to match?

Cheers,
Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Kent Gibson]

On Wed, Oct 09, 2019 at 08:22:11AM +0800, Kent Gibson wrote:
> On Wed, Oct 09, 2019 at 02:03:58AM +0200, Bartosz Golaszewski wrote:
> > śr., 9 paź 2019 o 01:56 Kent Gibson <warthog618@gmail.com> napisał(a):
> > 
> > But there's absolutely nothing wrong with writing code even if you're
> > not sure about the solution - it's even encouraged[1]. It's much
> > easier to discuss existing code than potential solutions.
> > 
> > [1] https://lkml.org/lkml/2000/8/25/132
> > 
> Exactly - show me your code.
> 
Safe to assume your code is in your topic/gpio-uapi-config branch?
I hope so, cos I've forked from that into github.com/warthog618/linux 
and have started working on it.

So far I've added pull up/down support to lineevent_create, and 
added default_values to the gpiohandle_config (for setting outputs).
Currently looking at implementing the actual set config, and sucking
on whether that is best done by refactoring bit out of
linehandle_create.

Will sort that into patches once it is in a fit state.
Feel free to poke me if I'm going off track.

Cheers,
Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Drew Fustini]

On Wed, Oct 09, 2019 at 02:55:24PM +0800, Kent Gibson wrote:
> Safe to assume your code is in your topic/gpio-uapi-config branch?
> I hope so, cos I've forked from that into github.com/warthog618/linux 
> and have started working on it.
> 
> So far I've added pull up/down support to lineevent_create, and 
> added default_values to the gpiohandle_config (for setting outputs).
> Currently looking at implementing the actual set config, and sucking
> on whether that is best done by refactoring bit out of
> linehandle_create.
> 
> Will sort that into patches once it is in a fit state.
> Feel free to poke me if I'm going off track.
> 
> Cheers,
> Kent.

Great to see that work on this capability is moving foward.  I also have
spare time so I'm eager to help however I can.

I had been working from the kernel.org torvalds/linux.git master to
create a patch and used 'git send-email' to post a patch.  It is
interesting that Bartosz and you have forks of linux where you created
topic branches.  I just added warthog618/linux as a remote so I can view
and try out your commits.

thanks,
drew

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Kent Gibson]

On Wed, Oct 09, 2019 at 02:57:27PM +0200, Drew Fustini wrote:
> 
> On Wed, Oct 09, 2019 at 02:55:24PM +0800, Kent Gibson wrote:
> > Safe to assume your code is in your topic/gpio-uapi-config branch?
> > I hope so, cos I've forked from that into github.com/warthog618/linux 
> > and have started working on it.
> > 
> > So far I've added pull up/down support to lineevent_create, and 
> > added default_values to the gpiohandle_config (for setting outputs).
> > Currently looking at implementing the actual set config, and sucking
> > on whether that is best done by refactoring bit out of
> > linehandle_create.
> > 
> > Will sort that into patches once it is in a fit state.
> > Feel free to poke me if I'm going off track.
> > 
> > Cheers,
> > Kent.
> 
> Great to see that work on this capability is moving foward.  I also have
> spare time so I'm eager to help however I can.
> 
> I had been working from the kernel.org torvalds/linux.git master to
> create a patch and used 'git send-email' to post a patch.  It is
> interesting that Bartosz and you have forks of linux where you created
> topic branches.  I just added warthog618/linux as a remote so I can view
> and try out your commits.
> 
Sorry that the branch is a munge of the pull up/down and set config changes.
And I have no idea how I'm going to sort this into a reasonable patch
sequence, but I'll burn that bridge when I get to it.

I'm reasonably happy with the state of the pull up/down.  That is
passing all my tests on gpio-mockup - other than the actual value being
pulled - but that is because gpio-mockup needs to be updated.
Which is next on the todo list. Along with building a Pi kernel to test
real hardware.  

I'd steer clear of the SET_CONFIG changes for the moment - those 
are totally untested.
But I guess you don't have any code to call the ioctl yet anyway...
That is slightly further down the todo list.

Cheers,
Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Drew Fustini]

On Wed, Oct 09, 2019 at 02:55:24PM +0800, Kent Gibson wrote:
> Safe to assume your code is in your topic/gpio-uapi-config branch?
> I hope so, cos I've forked from that into github.com/warthog618/linux 
> and have started working on it.

Do you also have a fork of libgpiod that you are working in?

In case it is of any use, I just posted the libgpiod patch for pull-up/down
flags that I had been using to test with.

I help maintain Adafruit_Blinka [1] so I would like try testing pull-up/down.
I already have a Raspberry Pi 3 booting a cross-compiled kernel with my (now
outdated) patch applied and a patched libgpiod.


[1] https://github.com/adafruit/Adafruit_Blinka

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Kent Gibson]

On Wed, Oct 09, 2019 at 03:30:37PM +0200, Drew Fustini wrote:
> On Wed, Oct 09, 2019 at 02:55:24PM +0800, Kent Gibson wrote:
> > Safe to assume your code is in your topic/gpio-uapi-config branch?
> > I hope so, cos I've forked from that into github.com/warthog618/linux 
> > and have started working on it.
> 
> Do you also have a fork of libgpiod that you are working in?
> 
Actually not yet - I'm using my Go equivalent of libgpiod to drive the
kernel for my tests.  That is also on github - it is in the uapi
directory of the feature/pud branch of my gpiod repo.

> In case it is of any use, I just posted the libgpiod patch for pull-up/down
> flags that I had been using to test with.
> 
> I help maintain Adafruit_Blinka [1] so I would like try testing pull-up/down.
> I already have a Raspberry Pi 3 booting a cross-compiled kernel with my (now
> outdated) patch applied and a patched libgpiod.
> 
Go nuts.  I hope to be testing on one of Pis, but I don't think I have a
recent kernel build handy, so I'll be a while.

Cheers,
Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Bartosz Golaszewski]

śr., 9 paź 2019 o 16:11 Kent Gibson <warthog618@gmail.com> napisał(a):
>
> On Wed, Oct 09, 2019 at 03:30:37PM +0200, Drew Fustini wrote:
> > On Wed, Oct 09, 2019 at 02:55:24PM +0800, Kent Gibson wrote:
> > > Safe to assume your code is in your topic/gpio-uapi-config branch?
> > > I hope so, cos I've forked from that into github.com/warthog618/linux
> > > and have started working on it.
> >
> > Do you also have a fork of libgpiod that you are working in?
> >
> Actually not yet - I'm using my Go equivalent of libgpiod to drive the
> kernel for my tests.  That is also on github - it is in the uapi
> directory of the feature/pud branch of my gpiod repo.
>
> > In case it is of any use, I just posted the libgpiod patch for pull-up/down
> > flags that I had been using to test with.
> >
> > I help maintain Adafruit_Blinka [1] so I would like try testing pull-up/down.
> > I already have a Raspberry Pi 3 booting a cross-compiled kernel with my (now
> > outdated) patch applied and a patched libgpiod.
> >
> Go nuts.  I hope to be testing on one of Pis, but I don't think I have a
> recent kernel build handy, so I'll be a while.
>
> Cheers,
> Kent.

Hey guys, just one thing: don't send patches as replies - just send a
new version as a separate series. Also: please aggregate all the
patches (pull-up/down, set config, gpio-mockup changes if any) into a
single series.

Bart

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Kent Gibson]

On Wed, Oct 09, 2019 at 05:50:00PM +0200, Bartosz Golaszewski wrote:
> śr., 9 paź 2019 o 16:11 Kent Gibson <warthog618@gmail.com> napisał(a):
> >
> > On Wed, Oct 09, 2019 at 03:30:37PM +0200, Drew Fustini wrote:
> > > On Wed, Oct 09, 2019 at 02:55:24PM +0800, Kent Gibson wrote:
> > > > Safe to assume your code is in your topic/gpio-uapi-config branch?
> > > > I hope so, cos I've forked from that into github.com/warthog618/linux
> > > > and have started working on it.
> > >
> > > Do you also have a fork of libgpiod that you are working in?
> > >
> > Actually not yet - I'm using my Go equivalent of libgpiod to drive the
> > kernel for my tests.  That is also on github - it is in the uapi
> > directory of the feature/pud branch of my gpiod repo.
> >
> > > In case it is of any use, I just posted the libgpiod patch for pull-up/down
> > > flags that I had been using to test with.
> > >
> > > I help maintain Adafruit_Blinka [1] so I would like try testing pull-up/down.
> > > I already have a Raspberry Pi 3 booting a cross-compiled kernel with my (now
> > > outdated) patch applied and a patched libgpiod.
> > >
> > Go nuts.  I hope to be testing on one of Pis, but I don't think I have a
> > recent kernel build handy, so I'll be a while.
> >
> > Cheers,
> > Kent.
> 
> Hey guys, just one thing: don't send patches as replies - just send a
> new version as a separate series. Also: please aggregate all the
> patches (pull-up/down, set config, gpio-mockup changes if any) into a
> single series.
> 
I think the only patch I've sent you was direct to you for feedback,
not to the list, and so not intended to be applied. And that was only
cos I thought you might prefer that to looking at the diffs on github.

Would it be ok to create two patch series - one for pull up/down and the
other for set config, if one still needs a lot more work?  
And is it ok to base them on your lineevent_create gotos tidy up
(f6cfbbe2950b2)?

Cheers,
Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Kent Gibson]

On Wed, Oct 09, 2019 at 03:30:37PM +0200, Drew Fustini wrote:
> On Wed, Oct 09, 2019 at 02:55:24PM +0800, Kent Gibson wrote:
> > Safe to assume your code is in your topic/gpio-uapi-config branch?
> > I hope so, cos I've forked from that into github.com/warthog618/linux 
> > and have started working on it.
> 
> Do you also have a fork of libgpiod that you are working in?
> 
> In case it is of any use, I just posted the libgpiod patch for pull-up/down
> flags that I had been using to test with.
> 
> I help maintain Adafruit_Blinka [1] so I would like try testing pull-up/down.
> I already have a Raspberry Pi 3 booting a cross-compiled kernel with my (now
> outdated) patch applied and a patched libgpiod.
> 
It is basically working for me on my Pi4:

pi@quoll:~ $ ./gpiodctl get gpiochip0 7
0
pi@quoll:~ $ ./gpiodctl get -u gpiochip0 7
1
pi@quoll:~ $ ./gpiodctl get gpiochip0 7
1
pi@quoll:~ $ ./gpiodctl get -d gpiochip0 7
0
pi@quoll:~ $ ./gpiodctl get gpiochip0 7
0

That is using the gpiodctl tool from my gpiod library.
My gpiod test suite also passes, but it doesn't do much to
exercise the UAPI.
I was intending to run my uapi test suite, which is more thorough,
but it turns out that only targets gpio-mockup, whereas my gpiod
test suite can target either.
Something else for the todo list.

Hopefully it is obvious what gpiodctl is doing. (-u sets the 
pull-up flag, -d sets the pull-down flag)
Looks like the pulls stick when the line is released, and the
subsequent get, without pull-up set, either doesn't clear the
pull-up/down or the line stays floating at the old pull level.
More investigation required, but that will have to wait til 
I get back to this later in the day.

Oh, and that is running on the rpi-5.3.3 kernel patched with everything
on my topic/gpio-uapi-config branch from 5.4-rc2 onward.

Cheers,
Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Drew Fustini]

On Thu, Oct 10, 2019 at 1:59 AM Kent Gibson <warthog618@gmail.com> wrote:
> It is basically working for me on my Pi4:
>
> pi@quoll:~ $ ./gpiodctl get gpiochip0 7
> 0
> pi@quoll:~ $ ./gpiodctl get -u gpiochip0 7
> 1
> pi@quoll:~ $ ./gpiodctl get gpiochip0 7
> 1
> pi@quoll:~ $ ./gpiodctl get -d gpiochip0 7
> 0
> pi@quoll:~ $ ./gpiodctl get gpiochip0 7
> 0
>
> That is using the gpiodctl tool from my gpiod library.
> My gpiod test suite also passes, but it doesn't do much to
> exercise the UAPI.
> I was intending to run my uapi test suite, which is more thorough,
> but it turns out that only targets gpio-mockup, whereas my gpiod
> test suite can target either.
> Something else for the todo list.
>
> Hopefully it is obvious what gpiodctl is doing. (-u sets the
> pull-up flag, -d sets the pull-down flag)
> Looks like the pulls stick when the line is released, and the
> subsequent get, without pull-up set, either doesn't clear the
> pull-up/down or the line stays floating at the old pull level.
> More investigation required, but that will have to wait til
> I get back to this later in the day.
>
> Oh, and that is running on the rpi-5.3.3 kernel patched with everything
> on my topic/gpio-uapi-config branch from 5.4-rc2 onward.

Thanks for sharing your results.

My Pi 3 had been running 5.3.0-v7+ from September 20th with my
pull-up/down patch (82fc38f6ab59).

I removed that patch and just cross-compiled 5.4-rc2 with
multi_v7_defconfig for the Pi3.

Are these the commits that I should apply from your topic branch?

bdc9696a27ed pull up/down requires explicit input mode in lineevent_create
14ee636232d4 disallow pull up/down on outputs
ce03bf5af1ec implement SET_CONFIG_IOCTL
f38b7554eb52 pull common validation code into linehandle_validate_flags
31c0aa53ffc3 Add default values for setting output
3c7ec03efcd9 add support for pull up/down to lineevent_create
99b85d1c26ea gpio: add new ioctl() to gpio chardev
82fc38f6ab59 gpio: expose pull-up/pull-down line flags to userspace
f6cfbbe2950b gpiolib: sanitize flags before allocating memory in
lineevent_create()

Thanks,
Drew

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Kent Gibson]

On Thu, Oct 10, 2019 at 09:47:35AM +0200, Drew Fustini wrote:
> On Thu, Oct 10, 2019 at 1:59 AM Kent Gibson <warthog618@gmail.com> wrote:
> > It is basically working for me on my Pi4:
> >
> > pi@quoll:~ $ ./gpiodctl get gpiochip0 7
> > 0
> > pi@quoll:~ $ ./gpiodctl get -u gpiochip0 7
> > 1
> > pi@quoll:~ $ ./gpiodctl get gpiochip0 7
> > 1
> > pi@quoll:~ $ ./gpiodctl get -d gpiochip0 7
> > 0
> > pi@quoll:~ $ ./gpiodctl get gpiochip0 7
> > 0
> >
> > That is using the gpiodctl tool from my gpiod library.
> > My gpiod test suite also passes, but it doesn't do much to
> > exercise the UAPI.
> > I was intending to run my uapi test suite, which is more thorough,
> > but it turns out that only targets gpio-mockup, whereas my gpiod
> > test suite can target either.
> > Something else for the todo list.
> >
> > Hopefully it is obvious what gpiodctl is doing. (-u sets the
> > pull-up flag, -d sets the pull-down flag)
> > Looks like the pulls stick when the line is released, and the
> > subsequent get, without pull-up set, either doesn't clear the
> > pull-up/down or the line stays floating at the old pull level.
> > More investigation required, but that will have to wait til
> > I get back to this later in the day.
> >
> > Oh, and that is running on the rpi-5.3.3 kernel patched with everything
> > on my topic/gpio-uapi-config branch from 5.4-rc2 onward.
> 
> Thanks for sharing your results.
> 
> My Pi 3 had been running 5.3.0-v7+ from September 20th with my
> pull-up/down patch (82fc38f6ab59).
> 
> I removed that patch and just cross-compiled 5.4-rc2 with
> multi_v7_defconfig for the Pi3.
> 
> Are these the commits that I should apply from your topic branch?
> 
> bdc9696a27ed pull up/down requires explicit input mode in lineevent_create
> 14ee636232d4 disallow pull up/down on outputs
> ce03bf5af1ec implement SET_CONFIG_IOCTL
> f38b7554eb52 pull common validation code into linehandle_validate_flags
> 31c0aa53ffc3 Add default values for setting output
> 3c7ec03efcd9 add support for pull up/down to lineevent_create
> 99b85d1c26ea gpio: add new ioctl() to gpio chardev
> 82fc38f6ab59 gpio: expose pull-up/pull-down line flags to userspace
> f6cfbbe2950b gpiolib: sanitize flags before allocating memory in
> lineevent_create()
> 
Those are the ones.

Plus there are now another 3:
625cd0a0df3ad9 actively disable bias on outputs
9d1f9db81b4dc4 actively disable bias on inputs when pull up/down not set
c6d4bf32c05189 add set_config to support pull up/down

Those add pull up/down support to gpio-mockup, and fix the stuck pulls 
I noted earlier - though they still remain applied until the next time
the line is requested.

9d1f9db81b4dc4 doesn't like being applied to the rpi kernel, I assume as
there have been conflicting changes between 5.3.3 and 5.4-rc2, so I
patched that one manually.

Cheers,
Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Kent Gibson]

On Thu, Oct 10, 2019 at 06:14:21PM +0800, Kent Gibson wrote:
> On Thu, Oct 10, 2019 at 09:47:35AM +0200, Drew Fustini wrote:
> > On Thu, Oct 10, 2019 at 1:59 AM Kent Gibson <warthog618@gmail.com> wrote:
> > > It is basically working for me on my Pi4:
> > >
> > > pi@quoll:~ $ ./gpiodctl get gpiochip0 7
> > > 0
> > > pi@quoll:~ $ ./gpiodctl get -u gpiochip0 7
> > > 1
> > > pi@quoll:~ $ ./gpiodctl get gpiochip0 7
> > > 1
> > > pi@quoll:~ $ ./gpiodctl get -d gpiochip0 7
> > > 0
> > > pi@quoll:~ $ ./gpiodctl get gpiochip0 7
> > > 0
> > >
> > > That is using the gpiodctl tool from my gpiod library.
> > > My gpiod test suite also passes, but it doesn't do much to
> > > exercise the UAPI.
> > > I was intending to run my uapi test suite, which is more thorough,
> > > but it turns out that only targets gpio-mockup, whereas my gpiod
> > > test suite can target either.
> > > Something else for the todo list.
> > >
> > > Hopefully it is obvious what gpiodctl is doing. (-u sets the
> > > pull-up flag, -d sets the pull-down flag)
> > > Looks like the pulls stick when the line is released, and the
> > > subsequent get, without pull-up set, either doesn't clear the
> > > pull-up/down or the line stays floating at the old pull level.
> > > More investigation required, but that will have to wait til
> > > I get back to this later in the day.
> > >
> > > Oh, and that is running on the rpi-5.3.3 kernel patched with everything
> > > on my topic/gpio-uapi-config branch from 5.4-rc2 onward.
> > 
> > Thanks for sharing your results.
> > 
> > My Pi 3 had been running 5.3.0-v7+ from September 20th with my
> > pull-up/down patch (82fc38f6ab59).
> > 
> > I removed that patch and just cross-compiled 5.4-rc2 with
> > multi_v7_defconfig for the Pi3.
> > 
> > Are these the commits that I should apply from your topic branch?
> > 
> > bdc9696a27ed pull up/down requires explicit input mode in lineevent_create
> > 14ee636232d4 disallow pull up/down on outputs
> > ce03bf5af1ec implement SET_CONFIG_IOCTL
> > f38b7554eb52 pull common validation code into linehandle_validate_flags
> > 31c0aa53ffc3 Add default values for setting output
> > 3c7ec03efcd9 add support for pull up/down to lineevent_create
> > 99b85d1c26ea gpio: add new ioctl() to gpio chardev
> > 82fc38f6ab59 gpio: expose pull-up/pull-down line flags to userspace
> > f6cfbbe2950b gpiolib: sanitize flags before allocating memory in
> > lineevent_create()
> > 
> Those are the ones.
> 
> Plus there are now another 3:
> 625cd0a0df3ad9 actively disable bias on outputs
> 9d1f9db81b4dc4 actively disable bias on inputs when pull up/down not set
> c6d4bf32c05189 add set_config to support pull up/down
> 
> Those add pull up/down support to gpio-mockup, and fix the stuck pulls 
> I noted earlier - though they still remain applied until the next time
> the line is requested.
> 
> 9d1f9db81b4dc4 doesn't like being applied to the rpi kernel, I assume as
> there have been conflicting changes between 5.3.3 and 5.4-rc2, so I
> patched that one manually.
> 
The disable bias changes I mentioned above are problematic - they break
ABI backward compatibility by explicitly disabling bias when neither
PULL_UP nor PULL_DOWN are set - which will be the case for legacy apps.

We really need to handle 4 possible bias states - None, Pull Up, Pull
Down and As Is, with the zero default being As Is.
So I intend to reinterpret the pull up and down flags as a two bit field
encoding the following:

0 - AsIs
1 - PullUp
2 - PullDown
3 - PullNone

Any objections?

Cheers,
Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Drew Fustini]

Hi Kent, that makes sense to me. Hopefully, Bartosz and/or Linus will
advise if this is an issue.

thanks,
drew

On Thu, Oct 10, 2019 at 1:17 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Thu, Oct 10, 2019 at 06:14:21PM +0800, Kent Gibson wrote:
> > On Thu, Oct 10, 2019 at 09:47:35AM +0200, Drew Fustini wrote:
> > > On Thu, Oct 10, 2019 at 1:59 AM Kent Gibson <warthog618@gmail.com> wrote:
> > > > It is basically working for me on my Pi4:
> > > >
> > > > pi@quoll:~ $ ./gpiodctl get gpiochip0 7
> > > > 0
> > > > pi@quoll:~ $ ./gpiodctl get -u gpiochip0 7
> > > > 1
> > > > pi@quoll:~ $ ./gpiodctl get gpiochip0 7
> > > > 1
> > > > pi@quoll:~ $ ./gpiodctl get -d gpiochip0 7
> > > > 0
> > > > pi@quoll:~ $ ./gpiodctl get gpiochip0 7
> > > > 0
> > > >
> > > > That is using the gpiodctl tool from my gpiod library.
> > > > My gpiod test suite also passes, but it doesn't do much to
> > > > exercise the UAPI.
> > > > I was intending to run my uapi test suite, which is more thorough,
> > > > but it turns out that only targets gpio-mockup, whereas my gpiod
> > > > test suite can target either.
> > > > Something else for the todo list.
> > > >
> > > > Hopefully it is obvious what gpiodctl is doing. (-u sets the
> > > > pull-up flag, -d sets the pull-down flag)
> > > > Looks like the pulls stick when the line is released, and the
> > > > subsequent get, without pull-up set, either doesn't clear the
> > > > pull-up/down or the line stays floating at the old pull level.
> > > > More investigation required, but that will have to wait til
> > > > I get back to this later in the day.
> > > >
> > > > Oh, and that is running on the rpi-5.3.3 kernel patched with everything
> > > > on my topic/gpio-uapi-config branch from 5.4-rc2 onward.
> > >
> > > Thanks for sharing your results.
> > >
> > > My Pi 3 had been running 5.3.0-v7+ from September 20th with my
> > > pull-up/down patch (82fc38f6ab59).
> > >
> > > I removed that patch and just cross-compiled 5.4-rc2 with
> > > multi_v7_defconfig for the Pi3.
> > >
> > > Are these the commits that I should apply from your topic branch?
> > >
> > > bdc9696a27ed pull up/down requires explicit input mode in lineevent_create
> > > 14ee636232d4 disallow pull up/down on outputs
> > > ce03bf5af1ec implement SET_CONFIG_IOCTL
> > > f38b7554eb52 pull common validation code into linehandle_validate_flags
> > > 31c0aa53ffc3 Add default values for setting output
> > > 3c7ec03efcd9 add support for pull up/down to lineevent_create
> > > 99b85d1c26ea gpio: add new ioctl() to gpio chardev
> > > 82fc38f6ab59 gpio: expose pull-up/pull-down line flags to userspace
> > > f6cfbbe2950b gpiolib: sanitize flags before allocating memory in
> > > lineevent_create()
> > >
> > Those are the ones.
> >
> > Plus there are now another 3:
> > 625cd0a0df3ad9 actively disable bias on outputs
> > 9d1f9db81b4dc4 actively disable bias on inputs when pull up/down not set
> > c6d4bf32c05189 add set_config to support pull up/down
> >
> > Those add pull up/down support to gpio-mockup, and fix the stuck pulls
> > I noted earlier - though they still remain applied until the next time
> > the line is requested.
> >
> > 9d1f9db81b4dc4 doesn't like being applied to the rpi kernel, I assume as
> > there have been conflicting changes between 5.3.3 and 5.4-rc2, so I
> > patched that one manually.
> >
> The disable bias changes I mentioned above are problematic - they break
> ABI backward compatibility by explicitly disabling bias when neither
> PULL_UP nor PULL_DOWN are set - which will be the case for legacy apps.
>
> We really need to handle 4 possible bias states - None, Pull Up, Pull
> Down and As Is, with the zero default being As Is.
> So I intend to reinterpret the pull up and down flags as a two bit field
> encoding the following:
>
> 0 - AsIs
> 1 - PullUp
> 2 - PullDown
> 3 - PullNone
>
> Any objections?
>
> Cheers,
> Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Drew Fustini]

Hi Kent,

I noticed there are some additional commits in your branch including:
7f0bcb88f583  pull up/down on output is actually a valid option

Do you know what the result would be when the line is an output?

I seem to recall some chips might do High-Z when some pull-up/down is
set and direction is out.

thanks,
drew

On Thu, Oct 10, 2019 at 12:14 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Thu, Oct 10, 2019 at 09:47:35AM +0200, Drew Fustini wrote:
> > On Thu, Oct 10, 2019 at 1:59 AM Kent Gibson <warthog618@gmail.com> wrote:
> > > It is basically working for me on my Pi4:
> > >
> > > pi@quoll:~ $ ./gpiodctl get gpiochip0 7
> > > 0
> > > pi@quoll:~ $ ./gpiodctl get -u gpiochip0 7
> > > 1
> > > pi@quoll:~ $ ./gpiodctl get gpiochip0 7
> > > 1
> > > pi@quoll:~ $ ./gpiodctl get -d gpiochip0 7
> > > 0
> > > pi@quoll:~ $ ./gpiodctl get gpiochip0 7
> > > 0
> > >
> > > That is using the gpiodctl tool from my gpiod library.
> > > My gpiod test suite also passes, but it doesn't do much to
> > > exercise the UAPI.
> > > I was intending to run my uapi test suite, which is more thorough,
> > > but it turns out that only targets gpio-mockup, whereas my gpiod
> > > test suite can target either.
> > > Something else for the todo list.
> > >
> > > Hopefully it is obvious what gpiodctl is doing. (-u sets the
> > > pull-up flag, -d sets the pull-down flag)
> > > Looks like the pulls stick when the line is released, and the
> > > subsequent get, without pull-up set, either doesn't clear the
> > > pull-up/down or the line stays floating at the old pull level.
> > > More investigation required, but that will have to wait til
> > > I get back to this later in the day.
> > >
> > > Oh, and that is running on the rpi-5.3.3 kernel patched with everything
> > > on my topic/gpio-uapi-config branch from 5.4-rc2 onward.
> >
> > Thanks for sharing your results.
> >
> > My Pi 3 had been running 5.3.0-v7+ from September 20th with my
> > pull-up/down patch (82fc38f6ab59).
> >
> > I removed that patch and just cross-compiled 5.4-rc2 with
> > multi_v7_defconfig for the Pi3.
> >
> > Are these the commits that I should apply from your topic branch?
> >
> > bdc9696a27ed pull up/down requires explicit input mode in lineevent_create
> > 14ee636232d4 disallow pull up/down on outputs
> > ce03bf5af1ec implement SET_CONFIG_IOCTL
> > f38b7554eb52 pull common validation code into linehandle_validate_flags
> > 31c0aa53ffc3 Add default values for setting output
> > 3c7ec03efcd9 add support for pull up/down to lineevent_create
> > 99b85d1c26ea gpio: add new ioctl() to gpio chardev
> > 82fc38f6ab59 gpio: expose pull-up/pull-down line flags to userspace
> > f6cfbbe2950b gpiolib: sanitize flags before allocating memory in
> > lineevent_create()
> >
> Those are the ones.
>
> Plus there are now another 3:
> 625cd0a0df3ad9 actively disable bias on outputs
> 9d1f9db81b4dc4 actively disable bias on inputs when pull up/down not set
> c6d4bf32c05189 add set_config to support pull up/down
>
> Those add pull up/down support to gpio-mockup, and fix the stuck pulls
> I noted earlier - though they still remain applied until the next time
> the line is requested.
>
> 9d1f9db81b4dc4 doesn't like being applied to the rpi kernel, I assume as
> there have been conflicting changes between 5.3.3 and 5.4-rc2, so I
> patched that one manually.
>
> Cheers,
> Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Kent Gibson]

On Fri, Oct 11, 2019 at 03:06:41PM +0200, Drew Fustini wrote:
> Hi Kent,
> 
> I noticed there are some additional commits in your branch including:
> 7f0bcb88f583  pull up/down on output is actually a valid option
> 
Yeah pulls make no sense for push pull lines, but while looking into
disabling bias I discovered that there are some chips that support
pull-up/down for open drain/source outputs.
It seemed reasonable to support them rather than reject them outright,
hence the reversal.

> Do you know what the result would be when the line is an output?
> 
Behaviour would depend on the driver and chip.
I'm assuming the user would only request combinations that make sense
and that the driver would reject any combinations that might have
unfortunate results.

I'm working on improving my git-fu to collect the pull changes from that
branch, and hope to mail out a patch set shortly.

Cheers,
Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Drew Fustini]

set pull-up/down flags in lineevent_create and add sanity checks

Check the pull-up/down flags in lineevent_create() and set the
corresponding bits.

Add sanity checks to make pull-up and pull-down flags mutually
exclusive and only valid when the line is an input.

Signed-off-by: Drew Fustini <drew@pdp7.com>
---
 drivers/gpio/gpiolib.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 646ae4cffe26..babc26267561 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -554,6 +554,20 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 	     (lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE)))
 		return -EINVAL;
 
+	/*
+	 * Do not allow PULL_UP & PULL_DOWN flags to be set as they are
+	 * contradictory.
+	 */
+	if ((lflags & GPIOHANDLE_REQUEST_PULL_UP) &&
+	    (lflags & GPIOHANDLE_REQUEST_PULL_DOWN))
+		return -EINVAL;
+
+	/* PULL_UP and PULL_DOWN flags only make sense for input mode. */
+	if (!(lflags & GPIOHANDLE_REQUEST_INPUT) &&
+	    ((lflags & GPIOHANDLE_REQUEST_PULL_UP) ||
+	     (lflags & GPIOHANDLE_REQUEST_PULL_DOWN)))
+		return -EINVAL;
+
 	lh = kzalloc(sizeof(*lh), GFP_KERNEL);
 	if (!lh)
 		return -ENOMEM;
@@ -941,6 +955,24 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 		goto out_free_label;
 	}
 
+	/*
+	 * Do not allow PULL_UP & PULL_DOWN flags to be set as they are
+	 * contradictory.
+	 */
+	if ((lflags & GPIOHANDLE_REQUEST_PULL_UP) &&
+	    (lflags & GPIOHANDLE_REQUEST_PULL_DOWN)) {
+		ret = -EINVAL;
+		goto out_free_label;
+	}
+
+	/* PULL_UP and PULL_DOWN flags only make sense for input mode. */
+	if (!(lflags & GPIOHANDLE_REQUEST_INPUT) &&
+	    ((lflags & GPIOHANDLE_REQUEST_PULL_UP) ||
+	     (lflags & GPIOHANDLE_REQUEST_PULL_DOWN))) {
+		ret = -EINVAL;
+		goto out_free_label;
+	}
+
 	desc = &gdev->descs[offset];
 	ret = gpiod_request(desc, le->label);
 	if (ret)
@@ -950,6 +982,10 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 
 	if (lflags & GPIOHANDLE_REQUEST_ACTIVE_LOW)
 		set_bit(FLAG_ACTIVE_LOW, &desc->flags);
+	if (lflags & GPIO_PULL_UP)
+		set_bit(FLAG_PULL_UP, &desc->flags);
+	else if (lflags & GPIO_PULL_DOWN)
+		set_bit(FLAG_PULL_DOWN, &desc->flags);
 
 	ret = gpiod_direction_input(desc);
 	if (ret)
-- 
2.20.1

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Drew Fustini]

On Wed, Oct 09, 2019 at 07:21:20AM +0800, Kent Gibson wrote:
> And while we're talking, does the gpio-mockup support pull up/down
> being set from the kernel side?

I was curious where this refers to the kernel or libgpiod?

I see there is:
libgpiod/tests/mockup/gpio-mockup.c

and:
linux/drivers/gpio/gpio-mockup.c

thanks,
drew

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Kent Gibson]

On Wed, Oct 09, 2019 at 03:57:38PM +0200, Drew Fustini wrote:
> On Wed, Oct 09, 2019 at 07:21:20AM +0800, Kent Gibson wrote:
> > And while we're talking, does the gpio-mockup support pull up/down
> > being set from the kernel side?
> 
> I was curious where this refers to the kernel or libgpiod?
> 
> I see there is:
> libgpiod/tests/mockup/gpio-mockup.c
> 
> and:
> linux/drivers/gpio/gpio-mockup.c
> 
The kernel one.  I was referring to being able to change the pull on a
line using the pull up/down flags.  It can be set using debugfs, but the
point here is to show that it can be set via the new flags as well.

Cheers,
Kent.

Re: [RFC] gpio: expose pull-up/pull-down line flags to userspace

[Drew Fustini]

On Tue, Oct 8, 2019 at 8:15 AM Kent Gibson <warthog618@gmail.com> wrote:
> 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.

Kent and Bart - thanks for the review.

I've added setting the pull-up/down flags in lineevent_create().  And
I have added sanity checks to make pull-up and pull-down flags
mutually exclusive and only valid when the line is an input.

I tried to use "git send-email" to reply to your message but it
doesn't work seem to have threaded correctly, despite using
"--in-reply-to"

I have only submitted kernel patches before when I was completing the
Eudyptula Challenge, so I wasn't sure how to best post a follow-up
patch to address review feedback.  Maybe starting a new thread with v2
of the RFC?

thanks,
drew