Re: svn commit: r368585 - in head: sys/dev/gpio sys/sys tools/test tools/test/gpioevents usr.sbin/gpioctl
FWIW I generally approved the code, but didn't have the time to review it
in detail. I got the impression people were at least generally happy from
the review.
Thanks so much for picking this up.
Warner
On Sat, Dec 12, 2020, 11:34 AM Ian Lepore wrote:
> Author: ian
> Date: Sat Dec 12 18:34:15 2020
> New Revision: 368585
> URL: https://svnweb.freebsd.org/changeset/base/368585
>
> Log:
> Provide userland notification of gpio pin changes ("userland gpio
> interrupts").
>
> This is an import of the Google Summer of Code 2018 project completed by
> Christian Kramer (and, sadly, ignored by us for two years now). The
> goals
> stated for that project were:
>
> FreeBSD already has support for interrupts implemented in the GPIO
> controller drivers of several SoCs, but there are no interfaces to
> take
> advantage of them out of user space yet. The goal of this work is to
> implement such an interface by providing descriptors which integrate
> with the common I/O system calls and multiplexing mechanisms.
>
> The initial imported code supports the following functionality:
>
>- A kernel driver that provides an interface to the user space; the
> existing gpioc(4) driver was enhanced with this functionality.
>- Implement support for the most common I/O system calls / multiplexing
> mechanisms:
>- read() Places the pin number on which the interrupt occurred in
> the
> buffer. Blocking and non-blocking behaviour supported.
>-poll()/select()
>-kqueue()
>-signal driven I/O. Posting SIGIO when the O_ASYNC was set.
>- Many-to-many relationship between pins and file descriptors.
>- A file descriptor can monitor several GPIO pins.
>- A GPIO pin can be monitored by multiple file descriptors.
>- Integration with gpioctl and libgpio.
>
> I added some fixes (mostly to locking) and feature enhancements on top of
> the original gsoc code. The feature ehancements allow the user to choose
> between detailed and summary event reporting. Detailed reporting
> provides
> a record describing each pin change event. Summary reporting provides
> the
> time of the first and last change of each pin, and a count of how many
> times
> it changed state since the last read(2) call. Another enhancement allows
> the recording of multiple state change events on multiple pins between
> each
> call to read(2) (the original code would track only a single event at a
> time).
>
> The phabricator review for these changes timed out without approval, but
> I
> cite it below anyway, because the review contains a series of diffs that
> show how I evolved the code from its original state in Christian's github
> repo for the gsoc project to what is being commited here. (In effect,
> the phab review extends the VC history back to the original code.)
>
> Submitted by: Christian Kramer
> Obtained from:https://github.com/ckraemer/freebsd/tree/gsoc2018
> Differential Revision:https://reviews.freebsd.org/D27398
>
> Added:
> head/tools/test/gpioevents/
> head/tools/test/gpioevents/Makefile (contents, props changed)
> head/tools/test/gpioevents/gpioevents.c (contents, props changed)
> Modified:
> head/sys/dev/gpio/gpiobus.c
> head/sys/dev/gpio/gpioc.c
> head/sys/sys/gpio.h
> head/tools/test/README
> head/usr.sbin/gpioctl/gpioctl.c
>
> Modified: head/sys/dev/gpio/gpiobus.c
>
> ==
> --- head/sys/dev/gpio/gpiobus.c Sat Dec 12 17:11:22 2020(r368584)
> +++ head/sys/dev/gpio/gpiobus.c Sat Dec 12 18:34:15 2020(r368585)
> @@ -143,6 +143,15 @@ gpio_check_flags(uint32_t caps, uint32_t flags)
> /* Cannot mix pull-up/pull-down together. */
> if (flags & GPIO_PIN_PULLUP && flags & GPIO_PIN_PULLDOWN)
> return (EINVAL);
> + /* Cannot mix output and interrupt flags together */
> + if (flags & GPIO_PIN_OUTPUT && flags & GPIO_INTR_MASK)
> + return (EINVAL);
> + /* Only one interrupt flag can be defined at once */
> + if ((flags & GPIO_INTR_MASK) & ((flags & GPIO_INTR_MASK) - 1))
> + return (EINVAL);
> + /* The interrupt attached flag cannot be set */
> + if (flags & GPIO_INTR_ATTACHED)
> + return (EINVAL);
>
> return (0);
> }
>
> Modified: head/sys/dev/gpio/gpioc.c
>
> ==
> --- head/sys/dev/gpio/gpioc.c Sat Dec 12 17:11:22 2020(r368584)
> +++ head/sys/dev/gpio/gpioc.c Sat Dec 12 18:34:15 2020(r368585)
> @@ -35,8 +35,15 @@ __FBSDID("$FreeBSD$");
> #include
> #include
> #include
> +#include
> +#include
> +#include
> +#include
> #include
> #include
> +#include
> +#include
> +#include
> #include
>
> #include
> @@ -47,30 +54,510 @@
svn commit: r368585 - in head: sys/dev/gpio sys/sys tools/test tools/test/gpioevents usr.sbin/gpioctl
Author: ian
Date: Sat Dec 12 18:34:15 2020
New Revision: 368585
URL: https://svnweb.freebsd.org/changeset/base/368585
Log:
Provide userland notification of gpio pin changes ("userland gpio
interrupts").
This is an import of the Google Summer of Code 2018 project completed by
Christian Kramer (and, sadly, ignored by us for two years now). The goals
stated for that project were:
FreeBSD already has support for interrupts implemented in the GPIO
controller drivers of several SoCs, but there are no interfaces to take
advantage of them out of user space yet. The goal of this work is to
implement such an interface by providing descriptors which integrate
with the common I/O system calls and multiplexing mechanisms.
The initial imported code supports the following functionality:
- A kernel driver that provides an interface to the user space; the
existing gpioc(4) driver was enhanced with this functionality.
- Implement support for the most common I/O system calls / multiplexing
mechanisms:
- read() Places the pin number on which the interrupt occurred in the
buffer. Blocking and non-blocking behaviour supported.
-poll()/select()
-kqueue()
-signal driven I/O. Posting SIGIO when the O_ASYNC was set.
- Many-to-many relationship between pins and file descriptors.
- A file descriptor can monitor several GPIO pins.
- A GPIO pin can be monitored by multiple file descriptors.
- Integration with gpioctl and libgpio.
I added some fixes (mostly to locking) and feature enhancements on top of
the original gsoc code. The feature ehancements allow the user to choose
between detailed and summary event reporting. Detailed reporting provides
a record describing each pin change event. Summary reporting provides the
time of the first and last change of each pin, and a count of how many times
it changed state since the last read(2) call. Another enhancement allows
the recording of multiple state change events on multiple pins between each
call to read(2) (the original code would track only a single event at a time).
The phabricator review for these changes timed out without approval, but I
cite it below anyway, because the review contains a series of diffs that
show how I evolved the code from its original state in Christian's github
repo for the gsoc project to what is being commited here. (In effect,
the phab review extends the VC history back to the original code.)
Submitted by: Christian Kramer
Obtained from:https://github.com/ckraemer/freebsd/tree/gsoc2018
Differential Revision:https://reviews.freebsd.org/D27398
Added:
head/tools/test/gpioevents/
head/tools/test/gpioevents/Makefile (contents, props changed)
head/tools/test/gpioevents/gpioevents.c (contents, props changed)
Modified:
head/sys/dev/gpio/gpiobus.c
head/sys/dev/gpio/gpioc.c
head/sys/sys/gpio.h
head/tools/test/README
head/usr.sbin/gpioctl/gpioctl.c
Modified: head/sys/dev/gpio/gpiobus.c
==
--- head/sys/dev/gpio/gpiobus.c Sat Dec 12 17:11:22 2020(r368584)
+++ head/sys/dev/gpio/gpiobus.c Sat Dec 12 18:34:15 2020(r368585)
@@ -143,6 +143,15 @@ gpio_check_flags(uint32_t caps, uint32_t flags)
/* Cannot mix pull-up/pull-down together. */
if (flags & GPIO_PIN_PULLUP && flags & GPIO_PIN_PULLDOWN)
return (EINVAL);
+ /* Cannot mix output and interrupt flags together */
+ if (flags & GPIO_PIN_OUTPUT && flags & GPIO_INTR_MASK)
+ return (EINVAL);
+ /* Only one interrupt flag can be defined at once */
+ if ((flags & GPIO_INTR_MASK) & ((flags & GPIO_INTR_MASK) - 1))
+ return (EINVAL);
+ /* The interrupt attached flag cannot be set */
+ if (flags & GPIO_INTR_ATTACHED)
+ return (EINVAL);
return (0);
}
Modified: head/sys/dev/gpio/gpioc.c
==
--- head/sys/dev/gpio/gpioc.c Sat Dec 12 17:11:22 2020(r368584)
+++ head/sys/dev/gpio/gpioc.c Sat Dec 12 18:34:15 2020(r368585)
@@ -35,8 +35,15 @@ __FBSDID("$FreeBSD$");
#include
#include
#include
+#include
+#include
+#include
+#include
#include
#include
+#include
+#include
+#include
#include
#include
@@ -47,30 +54,510 @@ __FBSDID("$FreeBSD$");
#undef GPIOC_DEBUG
#ifdef GPIOC_DEBUG
#define dprintf printf
+#define ddevice_printf device_printf
#else
#define dprintf(x, arg...)
+#define ddevice_printf(dev, x, arg...)
#endif
-static int gpioc_probe(device_t dev);
-static int gpioc_attach(device_t dev);
-static int gpioc_detach(device_t dev);
+struct gpioc_softc {
+ device_tsc_dev; /* gpiocX dev */
+ device_tsc_pdev;/*
