Re: GTE - The hardware timestamping engine
On Mon, Mar 22, 2021 at 09:09:50PM -0700, Dipen Patel wrote: > > > On 3/22/21 7:59 PM, Kent Gibson wrote: > > On Mon, Mar 22, 2021 at 06:53:10PM -0700, Dipen Patel wrote: > >> > >> > >> On 3/22/21 5:32 PM, Kent Gibson wrote: > >>> On Mon, Mar 22, 2021 at 01:21:46PM -0700, Dipen Patel wrote: > >>>> Hi Linus and Kent, > >>>> > > > > [snip] > > > >>> In response to all your comments above... > >>> > >>> Firstly, I'm not suggesting that other kernel modules would use the > >>> cdev lineevents, only that they would use the same mechanism that > >>> gpiolib-cdev would use to timestamp the lineevents for userspace. > >>> > >> Sure, I just wanted to mention the different scenarios and wanted to know > >> how can we fit all those together. Having said that, shouldn't this serve > >> an opportunity to extend the linevent framework to accommodate kernel > >> drivers as a clients? > >> > >> If we can't, then there is a risk of duplicating lineevent mechanism in all > >> of those kernel drivers or at least in GTE framework/infrastructure as far > >> as GPIO related GTE part is concerned. > >> > > > > In-kernel the lineevents are just IRQs so anything needing a "lineevent" > > can request the IRQ directly. Or am I missing something? > > > > In the GPIO context, I meant we can extend line_event_timestamp to kernel > drivers as well in that way, both userspace and kernel drivers requesting > particular GPIO for the hardware timestamp would be managed by same > lineevent_* infrastructure from the gpiolib. Something like lineevent_create > version of the kernel drivers, so if they need hardware timestamp on the > GPIO line, they can request with some flags. In that way, GTE can leverage > linevent* codes from gpiolib to cover its both the GPIO related use cases i.e. > userspace app and kernel drivers. > I still don't see what that gives you that is better than an IRQ and a function to provide the timestamp for that IRQ. What specific features of a lineevent are you after? The gpiolib-cdev code is there to provide a palettable API for userspace, and the bulk of that code is specific to the userspace API. Reusing that code for clients within the kernel is just introducing pointless overhead when they can get what they need more directly. There may be a case for some additional gpiolib/irq helper functions, but I don't see gpiolib-cdev as a good fit for that role. > >>> As to that mechanism, my current thinking is that the approach of > >>> associating GTE event FIFO entries with particular physical IRQ events is > >>> problematic, as keeping the two in sync would be difficult, if not > >>> impossible. > >>> > >>> A more robust approach is to ignore the physical IRQs and instead service > >>> the GTE event FIFO, generating IRQs from those events in software - > >>> essentially a pre-timestamped IRQ. The IRQ framework could provide the > >>> timestamping functionality, equivalent to line_event_timestamp(), for > >>> the IRQ handler/thread and in this case provide the timestamp from the GTE > >>> event. > >>> > >> > >> I have not fully understood above two paragraphs (except about > >> lineevent_event_timestamp related part). > >> > >> I have no idea what it means to "ignore the physical IRQs and service the > >> GTE event FIFO". Just like GPIO clients, there could be IRQ clients which > >> want to monitor certain IRQ line, like ethernet driver wanted to retrieve > >> timestamp for its IRQ line and so on. > >> > > > > I mean that in the IRQ framework, rather than enabling the physical IRQ > > line it would leave that masked and would instead enable the FIFO line to > > service the FIFO, configure the GTE to generate the events for that > > line, and then generate IRQs in response to the FIFO events. > > That way the client requesting the IRQ is guaranteed to only receive an > > IRQ that corresponds to a GTE FIFO event and the timestamp stored in the > > IRQ framework would match. > > > > I do not think we need to do such things, for example, below is > the rough sequence how GTE can notify its clients be it GPIO or IRQ > lines. I believe this will also help understand better ongoing GPIO > discussions. > > 1. Configure GTE FIFO watermark or threshold, lets assume 1, i.e >generate GTE interrupt when FIFO depth is 1. > 2. In the GTE ISR or ISR thread, drain internal FIFO entr
Re: GTE - The hardware timestamping engine
On Mon, Mar 22, 2021 at 06:53:10PM -0700, Dipen Patel wrote: > > > On 3/22/21 5:32 PM, Kent Gibson wrote: > > On Mon, Mar 22, 2021 at 01:21:46PM -0700, Dipen Patel wrote: > >> Hi Linus and Kent, > >> [snip] > > In response to all your comments above... > > > > Firstly, I'm not suggesting that other kernel modules would use the > > cdev lineevents, only that they would use the same mechanism that > > gpiolib-cdev would use to timestamp the lineevents for userspace. > > > Sure, I just wanted to mention the different scenarios and wanted to know > how can we fit all those together. Having said that, shouldn't this serve > an opportunity to extend the linevent framework to accommodate kernel > drivers as a clients? > > If we can't, then there is a risk of duplicating lineevent mechanism in all > of those kernel drivers or at least in GTE framework/infrastructure as far > as GPIO related GTE part is concerned. > In-kernel the lineevents are just IRQs so anything needing a "lineevent" can request the IRQ directly. Or am I missing something? > > As to that mechanism, my current thinking is that the approach of > > associating GTE event FIFO entries with particular physical IRQ events is > > problematic, as keeping the two in sync would be difficult, if not > > impossible. > > > > A more robust approach is to ignore the physical IRQs and instead service > > the GTE event FIFO, generating IRQs from those events in software - > > essentially a pre-timestamped IRQ. The IRQ framework could provide the > > timestamping functionality, equivalent to line_event_timestamp(), for > > the IRQ handler/thread and in this case provide the timestamp from the GTE > > event. > > > > I have not fully understood above two paragraphs (except about > lineevent_event_timestamp related part). > > I have no idea what it means to "ignore the physical IRQs and service the > GTE event FIFO". Just like GPIO clients, there could be IRQ clients which > want to monitor certain IRQ line, like ethernet driver wanted to retrieve > timestamp for its IRQ line and so on. > I mean that in the IRQ framework, rather than enabling the physical IRQ line it would leave that masked and would instead enable the FIFO line to service the FIFO, configure the GTE to generate the events for that line, and then generate IRQs in response to the FIFO events. That way the client requesting the IRQ is guaranteed to only receive an IRQ that corresponds to a GTE FIFO event and the timestamp stored in the IRQ framework would match. And that is what I mean by this being an IRQ feature. We need feedback from the IRQ guys as to whether that makes sense to them. Cheers, Kent.
Re: GTE - The hardware timestamping engine
On Mon, Mar 22, 2021 at 01:21:46PM -0700, Dipen Patel wrote:
> Hi Linus and Kent,
>
> Thanks you so much for your valuable inputs and time, Please see below, my
> follow ups.
>
> On 3/21/21 11:00 PM, Kent Gibson wrote:
> > On Sat, Mar 20, 2021 at 12:56:36PM +0100, Linus Walleij wrote:
> >> Hi Dipen,
> >>
> >> thanks for your mail!
> >>
> >> I involved some other kernel people to get some discussion.
> >> I think Kent Gibson can be of great help because he is using
> >> GPIOs with high precision.
> >>
> >
> > Actually I just extended the cdev uAPI to provide the REALTIME option,
> > which was the event clock until we changed to MONOTONIC in Linux 5.7,
> > as there were some users that were requiring the REALTIME clock.
> >
> >> We actually discussed this a bit when adding support for
> >> realtime timestamps.
> >>
> >> On Wed, Mar 17, 2021 at 11:29 PM Dipen Patel wrote:
> >>
> >>> Nvidia Tegra SoCs have generic timestamping engine (GTE) hardware module
> >>> which
> >>> can monitor SoC signals like IRQ lines and GPIO lines for state change,
> >>> upon
> >>> detecting the change, it can timestamp and store in its internal hardware
> >>> FIFO.
> >>> The advantage of the GTE module can be realized in applications like
> >>> robotics
> >>> or autonomous vehicle where it can help record events with precise
> >>> timestamp.
> >>
> >> That sounds very useful.
> >>
> >
> > Indeed - it could remove the latency and jitter that results from
> > timestamping events in the IRQ handler.
> >
> >> Certainly the kernel shall be able to handle this.
> >>
> >>>
> >>> For GPIO:
> >>>
> >>> 1. GPIO has to be configured as input and IRQ must be enabled.
> >>> 2. Ask GPIO controller driver to set corresponding timestamp bit in the
> >>> specified GPIO config register.
> >>> 3. Translate GPIO specified by the client to its internal bitmap.
> >>> 3.a For example, If client specifies GPIO line 31, it could be bit 13 of
> >>> GTE
> >>> register.
> >>> 4. Set internal bits to enable monitoring in GTE module
> >>> 5. Additionally GTE driver can open up lanes for the user space
> >>> application
> >>> as a client and can send timestamping events directly to the
> >>> application.
> >>
> >> I have some concerns:
> >>
> >> 1. GPIO should for all professional applications be used with the character
> >> device /dev/gpiochipN, under no circumstances shall the old sysfs
> >> ABI be used for this. In this case it is necessary because the
> >> character device provides events in a FIFO to userspace, which is
> >> what we need.
> >>
> >
> > The cdev uAPI would certainly be the most sensible place to expose
> > this to userspace - its line events being a direct analog to what the GTE
> > provides.
> >
> >> The timestamp provided to userspace is an opaque 64bit
> >> unsigned value. I suppose we assume it is monotonic but
> >> you can actually augment the semantics for your specific
> >> stamp, as long as 64 bits is gonna work.
> >>
> >> 2. The timestamp for the chardev is currently obtained in
> >> drivers/gpio/gpiolib-cdev.c like this:
> >>
> >> static u64 line_event_timestamp(struct line *line)
> >> {
> >> if (test_bit(FLAG_EVENT_CLOCK_REALTIME, >desc->flags))
> >> return ktime_get_real_ns();
> >>
> >> return ktime_get_ns();
> >> }
> >>
> >> What you want to do is to add a new flag for hardware timestamps
> >> and use that if available. FLAG_EVENT_CLOCK_HARDWARE?
> >> FLAG_EVENT_CLOCK_NATIVE?
> >>
> >
> > HARDWARE looks better to me, as NATIVE is more vague.
> >
> >> Then you need to figure out a mechanism so we can obtain
> >> the right timestamp from the hardware event right here,
> >> you can hook into the GPIO driver if need be, we can
> >> figure out the gpio_chip for a certain line for sure.
> >>
> >
> > Firstly, line_event_timestamp() is called from the IRQ handler context.
> > That is obviously more constraining than if it were only called from the
> > IRQ thread. If the GTE is providing the timestamp then that could be
> >
Re: GTE - The hardware timestamping engine
On Sat, Mar 20, 2021 at 12:56:36PM +0100, Linus Walleij wrote:
> Hi Dipen,
>
> thanks for your mail!
>
> I involved some other kernel people to get some discussion.
> I think Kent Gibson can be of great help because he is using
> GPIOs with high precision.
>
Actually I just extended the cdev uAPI to provide the REALTIME option,
which was the event clock until we changed to MONOTONIC in Linux 5.7,
as there were some users that were requiring the REALTIME clock.
> We actually discussed this a bit when adding support for
> realtime timestamps.
>
> On Wed, Mar 17, 2021 at 11:29 PM Dipen Patel wrote:
>
> > Nvidia Tegra SoCs have generic timestamping engine (GTE) hardware module
> > which
> > can monitor SoC signals like IRQ lines and GPIO lines for state change, upon
> > detecting the change, it can timestamp and store in its internal hardware
> > FIFO.
> > The advantage of the GTE module can be realized in applications like
> > robotics
> > or autonomous vehicle where it can help record events with precise
> > timestamp.
>
> That sounds very useful.
>
Indeed - it could remove the latency and jitter that results from
timestamping events in the IRQ handler.
> Certainly the kernel shall be able to handle this.
>
> >
> > For GPIO:
> >
> > 1. GPIO has to be configured as input and IRQ must be enabled.
> > 2. Ask GPIO controller driver to set corresponding timestamp bit in the
> > specified GPIO config register.
> > 3. Translate GPIO specified by the client to its internal bitmap.
> > 3.a For example, If client specifies GPIO line 31, it could be bit 13 of GTE
> > register.
> > 4. Set internal bits to enable monitoring in GTE module
> > 5. Additionally GTE driver can open up lanes for the user space application
> > as a client and can send timestamping events directly to the
> > application.
>
> I have some concerns:
>
> 1. GPIO should for all professional applications be used with the character
> device /dev/gpiochipN, under no circumstances shall the old sysfs
> ABI be used for this. In this case it is necessary because the
> character device provides events in a FIFO to userspace, which is
> what we need.
>
The cdev uAPI would certainly be the most sensible place to expose
this to userspace - its line events being a direct analog to what the GTE
provides.
> The timestamp provided to userspace is an opaque 64bit
> unsigned value. I suppose we assume it is monotonic but
> you can actually augment the semantics for your specific
> stamp, as long as 64 bits is gonna work.
>
> 2. The timestamp for the chardev is currently obtained in
> drivers/gpio/gpiolib-cdev.c like this:
>
> static u64 line_event_timestamp(struct line *line)
> {
> if (test_bit(FLAG_EVENT_CLOCK_REALTIME, >desc->flags))
> return ktime_get_real_ns();
>
> return ktime_get_ns();
> }
>
> What you want to do is to add a new flag for hardware timestamps
> and use that if available. FLAG_EVENT_CLOCK_HARDWARE?
> FLAG_EVENT_CLOCK_NATIVE?
>
HARDWARE looks better to me, as NATIVE is more vague.
> Then you need to figure out a mechanism so we can obtain
> the right timestamp from the hardware event right here,
> you can hook into the GPIO driver if need be, we can
> figure out the gpio_chip for a certain line for sure.
>
Firstly, line_event_timestamp() is called from the IRQ handler context.
That is obviously more constraining than if it were only called from the
IRQ thread. If the GTE is providing the timestamp then that could be
put off until the IRQ thread.
So you probably want to refactor line_event_timestamp() into two flavours
- one for IRQ handler that returns 0 if HARDWARE is set, and the other for
IRQ thread, where there is already a fallback call to
line_event_timestamp() for the nested threaded interrupt case, that gets
the timestamp from the GTE.
But my primary concern here would be keeping the two event FIFOs (GTE and
cdev) in sync. Masking and unmasking in hardware and the kernel needs to
be coordinated to prevent races that would result in sync loss.
So this probably needs to be configured in the GTE driver via the irq
path, rather than pinctrl?
Is every event detected by the GTE guaranteed to trigger an interrupt in
the kernel?
How to handle GTE FIFO overflows? Can they be detected or prevented?
> So you first need to augment the userspace
> ABI and the character device code to add this. See
> commit 26d060e47e25f2c715a1b2c48fea391f67907a30
> "gpiolib: cdev: allow edge event timestamps to be configured as REALTIME"
> by Kent Gibson to see what needs to be done.
>
You should also extend gpio_v2_line_flags_validate
Re: [PATCH] selftests: gpio: update .gitignore
On Wed, Feb 24, 2021 at 07:53:16PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski
>
> The executable that we build for GPIO selftests was renamed to
> gpio-mockup-cdev. Let's update .gitignore so that we don't show it
> as an untracked file.
>
> Fixes: 8bc395a6a2e2 ("selftests: gpio: rework and simplify test
> implementation")
> Signed-off-by: Bartosz Golaszewski
> ---
> tools/testing/selftests/gpio/.gitignore | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/gpio/.gitignore
> b/tools/testing/selftests/gpio/.gitignore
> index 4c69408f3e84..a4969f7ee020 100644
> --- a/tools/testing/selftests/gpio/.gitignore
> +++ b/tools/testing/selftests/gpio/.gitignore
> @@ -1,2 +1,2 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -gpio-mockup-chardev
> +gpio-mockup-cdev
> --
Thanks for picking this one up - I missed it as I don't build the
selftests in the same environment that I edit them in, and overlooked
that there was a .gitignore in play.
Reviewed-by: Kent Gibson
Cheers,
Kent.
Re: [PATCH 8/8] gpio: sim: new testing module
On Sat, Jan 30, 2021 at 09:37:55PM +0100, Bartosz Golaszewski wrote: > On Fri, Jan 29, 2021 at 4:57 PM Andy Shevchenko > wrote: > > > > On Fri, Jan 29, 2021 at 02:46:24PM +0100, Bartosz Golaszewski wrote: > > > From: Bartosz Golaszewski > > ... > > [snip] > > Honestly, I don't like the idea of Yet Another (custom) Parser in the > > kernel. > > > > Have you investigated existing parsers? We have cmdline.c, > > gpio-aggregator.c, > > etc. Besides the fact of test cases which are absent here. And who knows > > what > > we allow to be entered. > > > > Yes, I looked all around the kernel to find something I could reuse > but failed to find anything useful for this particular purpose. If you > have something you could point me towards, I'm open to alternatives. > > Once we agree on the form of the module, I'll port self-tests to using > it instead of gpio-mockup, so we'll have some tests in the tree. > Given the existing selftests focus on testing the gpio-mockup itself, it would be more appropriate that you add separate tests for gpio-sim. As an end user I'm interested in the concrete example of driving gpio-sim that selftests would provide, so I'm looking forward to seeing that. Cheers, Kent.
[PATCH] gpiolib: cdev: clear debounce period if line set to output
When set_config changes a line from input to output debounce is
implicitly disabled, as debounce makes no sense for outputs, but the
debounce period is not being cleared and is still reported in the
line info.
So clear the debounce period when the debouncer is stopped in
edge_detector_stop().
Fixed: 65cff7047640 ("gpiolib: cdev: support setting debounce")
Signed-off-by: Kent Gibson
---
drivers/gpio/gpiolib-cdev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 12b679ca552c..3551aaf5a361 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -776,6 +776,8 @@ static void edge_detector_stop(struct line *line)
cancel_delayed_work_sync(>work);
WRITE_ONCE(line->sw_debounced, 0);
WRITE_ONCE(line->eflags, 0);
+ if (line->desc)
+ WRITE_ONCE(line->desc->debounce_period_us, 0);
/* do not change line->level - see comment in debounced_value() */
}
--
2.30.0
[PATCH] gpio: uapi: fix line info flags description
The description of the flags field of the struct gpio_v2_line_info
mentions "the GPIO lines" while the info only applies to an individual
GPIO line. This was accidentally changed from "the GPIO line" during
formatting improvements.
Reword to "this GPIO line" to clarify and to be consistent with other
struct gpio_v2_line_info fields.
Fixes: 2cc522d3931b ("gpio: uapi: kernel-doc formatting improvements")
Signed-off-by: Kent Gibson
---
include/uapi/linux/gpio.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index e4eb0b8c5cf9..7a6f7948a7df 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -212,7 +212,7 @@ struct gpio_v2_line_request {
* @offset: the local offset on this GPIO chip, fill this in when
* requesting the line information from the kernel
* @num_attrs: the number of attributes in @attrs
- * @flags: flags for the GPIO lines, with values from
+ * @flags: flags for this GPIO line, with values from
* gpio_v2_line_flag, such as %GPIO_V2_LINE_FLAG_ACTIVE_LOW,
* %GPIO_V2_LINE_FLAG_OUTPUT etc, added together.
* @attrs: the configuration attributes associated with the line
--
2.30.0
[PATCH v3 6/7] selftests: gpio: port to GPIO uAPI v2
Add a port to the GPIO uAPI v2 interface and make it the default.
Signed-off-by: Kent Gibson
Acked-by: Linus Walleij
Reviewed-by: Bartosz Golaszewski
---
.../testing/selftests/gpio/gpio-mockup-cdev.c | 75 +--
tools/testing/selftests/gpio/gpio-mockup.sh | 11 ++-
2 files changed, 76 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/gpio/gpio-mockup-cdev.c
b/tools/testing/selftests/gpio/gpio-mockup-cdev.c
index 3a7fc3ac1349..e83eac71621a 100644
--- a/tools/testing/selftests/gpio/gpio-mockup-cdev.c
+++ b/tools/testing/selftests/gpio/gpio-mockup-cdev.c
@@ -18,6 +18,44 @@
#define CONSUMER "gpio-mockup-cdev"
+static int request_line_v2(int cfd, unsigned int offset,
+ uint64_t flags, unsigned int val)
+{
+ struct gpio_v2_line_request req;
+ int ret;
+
+ memset(, 0, sizeof(req));
+ req.num_lines = 1;
+ req.offsets[0] = offset;
+ req.config.flags = flags;
+ strcpy(req.consumer, CONSUMER);
+ if (flags & GPIO_V2_LINE_FLAG_OUTPUT) {
+ req.config.num_attrs = 1;
+ req.config.attrs[0].mask = 1;
+ req.config.attrs[0].attr.id =
GPIO_V2_LINE_ATTR_ID_OUTPUT_VALUES;
+ if (val)
+ req.config.attrs[0].attr.values = 1;
+ }
+ ret = ioctl(cfd, GPIO_V2_GET_LINE_IOCTL, );
+ if (ret == -1)
+ return -errno;
+ return req.fd;
+}
+
+
+static int get_value_v2(int lfd)
+{
+ struct gpio_v2_line_values vals;
+ int ret;
+
+ memset(, 0, sizeof(vals));
+ vals.mask = 1;
+ ret = ioctl(lfd, GPIO_V2_LINE_GET_VALUES_IOCTL, );
+ if (ret == -1)
+ return -errno;
+ return vals.bits & 0x1;
+}
+
static int request_line_v1(int cfd, unsigned int offset,
uint32_t flags, unsigned int val)
{
@@ -57,6 +95,7 @@ static void usage(char *prog)
printf(" (default is to leave bias unchanged):\n");
printf("-l: set line active low (default is active high)\n");
printf("-s: set line value (default is to get line value)\n");
+ printf("-u: uAPI version to use (default is 2)\n");
exit(-1);
}
@@ -78,29 +117,42 @@ int main(int argc, char *argv[])
{
char *chip;
int opt, ret, cfd, lfd;
- unsigned int offset, val;
+ unsigned int offset, val, abiv;
uint32_t flags_v1;
+ uint64_t flags_v2;
+ abiv = 2;
ret = 0;
flags_v1 = GPIOHANDLE_REQUEST_INPUT;
+ flags_v2 = GPIO_V2_LINE_FLAG_INPUT;
while ((opt = getopt(argc, argv, "lb:s:u:")) != -1) {
switch (opt) {
case 'l':
flags_v1 |= GPIOHANDLE_REQUEST_ACTIVE_LOW;
+ flags_v2 |= GPIO_V2_LINE_FLAG_ACTIVE_LOW;
break;
case 'b':
- if (strcmp("pull-up", optarg) == 0)
+ if (strcmp("pull-up", optarg) == 0) {
flags_v1 |= GPIOHANDLE_REQUEST_BIAS_PULL_UP;
- else if (strcmp("pull-down", optarg) == 0)
+ flags_v2 |= GPIO_V2_LINE_FLAG_BIAS_PULL_UP;
+ } else if (strcmp("pull-down", optarg) == 0) {
flags_v1 |= GPIOHANDLE_REQUEST_BIAS_PULL_DOWN;
- else if (strcmp("disabled", optarg) == 0)
+ flags_v2 |= GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN;
+ } else if (strcmp("disabled", optarg) == 0) {
flags_v1 |= GPIOHANDLE_REQUEST_BIAS_DISABLE;
+ flags_v2 |= GPIO_V2_LINE_FLAG_BIAS_DISABLED;
+ }
break;
case 's':
val = atoi(optarg);
flags_v1 &= ~GPIOHANDLE_REQUEST_INPUT;
flags_v1 |= GPIOHANDLE_REQUEST_OUTPUT;
+ flags_v2 &= ~GPIO_V2_LINE_FLAG_INPUT;
+ flags_v2 |= GPIO_V2_LINE_FLAG_OUTPUT;
+ break;
+ case 'u':
+ abiv = atoi(optarg);
break;
default:
usage(argv[0]);
@@ -119,7 +171,10 @@ int main(int argc, char *argv[])
return -errno;
}
- lfd = request_line_v1(cfd, offset, flags_v1, val);
+ if (abiv == 1)
+ lfd = request_line_v1(cfd, offset, flags_v1, val);
+ else
+ lfd = request_line_v2(cfd, offset, flags_v2, val);
close(cfd);
@@ -128,10 +183,14 @@ int main(int argc, char *argv[])
return lfd;
}
- if (flags_v1 & GPIOHANDLE_REQUE
[PATCH v3 7/7] selftests: gpio: add CONFIG_GPIO_CDEV to config
GPIO CDEV is now optional and required for the selftests so add it to the config. Signed-off-by: Kent Gibson Acked-by: Linus Walleij Reviewed-by: Bartosz Golaszewski --- tools/testing/selftests/gpio/config | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/gpio/config b/tools/testing/selftests/gpio/config index abaa6902b7b6..ce100342c20b 100644 --- a/tools/testing/selftests/gpio/config +++ b/tools/testing/selftests/gpio/config @@ -1,2 +1,3 @@ CONFIG_GPIOLIB=y +CONFIG_GPIO_CDEV=y CONFIG_GPIO_MOCKUP=m -- 2.30.0
[PATCH v3 5/7] tools: gpio: remove uAPI v1 code no longer used by selftests
gpio-mockup-chardev helper has been obsoleted and removed, so also remove
the tools/gpio code that it, and nothing else, was using.
Signed-off-by: Kent Gibson
Acked-by: Linus Walleij
Reviewed-by: Bartosz Golaszewski
---
tools/gpio/gpio-utils.c | 89 -
tools/gpio/gpio-utils.h | 6 ---
2 files changed, 95 deletions(-)
diff --git a/tools/gpio/gpio-utils.c b/tools/gpio/gpio-utils.c
index 37187e056c8b..1639b4d832cd 100644
--- a/tools/gpio/gpio-utils.c
+++ b/tools/gpio/gpio-utils.c
@@ -32,74 +32,6 @@
* following api will request gpio lines, do the operation and then
* release these lines.
*/
-/**
- * gpiotools_request_linehandle() - request gpio lines in a gpiochip
- * @device_name: The name of gpiochip without prefix "/dev/",
- * such as "gpiochip0"
- * @lines: An array desired lines, specified by offset
- * index for the associated GPIO device.
- * @num_lines: The number of lines to request.
- * @flag: The new flag for requsted gpio. Reference
- * "linux/gpio.h" for the meaning of flag.
- * @data: Default value will be set to gpio when flag is
- * GPIOHANDLE_REQUEST_OUTPUT.
- * @consumer_label:The name of consumer, such as "sysfs",
- * "powerkey". This is useful for other users to
- * know who is using.
- *
- * Request gpio lines through the ioctl provided by chardev. User
- * could call gpiotools_set_values() and gpiotools_get_values() to
- * read and write respectively through the returned fd. Call
- * gpiotools_release_linehandle() to release these lines after that.
- *
- * Return: On success return the fd;
- * On failure return the errno.
- */
-int gpiotools_request_linehandle(const char *device_name, unsigned int *lines,
-unsigned int num_lines, unsigned int flag,
-struct gpiohandle_data *data,
-const char *consumer_label)
-{
- struct gpiohandle_request req;
- char *chrdev_name;
- int fd;
- int i;
- int ret;
-
- ret = asprintf(_name, "/dev/%s", device_name);
- if (ret < 0)
- return -ENOMEM;
-
- fd = open(chrdev_name, 0);
- if (fd == -1) {
- ret = -errno;
- fprintf(stderr, "Failed to open %s, %s\n",
- chrdev_name, strerror(errno));
- goto exit_free_name;
- }
-
- for (i = 0; i < num_lines; i++)
- req.lineoffsets[i] = lines[i];
-
- req.flags = flag;
- strcpy(req.consumer_label, consumer_label);
- req.lines = num_lines;
- if (flag & GPIOHANDLE_REQUEST_OUTPUT)
- memcpy(req.default_values, data, sizeof(req.default_values));
-
- ret = ioctl(fd, GPIO_GET_LINEHANDLE_IOCTL, );
- if (ret == -1) {
- ret = -errno;
- fprintf(stderr, "Failed to issue %s (%d), %s\n",
- "GPIO_GET_LINEHANDLE_IOCTL", ret, strerror(errno));
- }
-
- if (close(fd) == -1)
- perror("Failed to close GPIO character device file");
-exit_free_name:
- free(chrdev_name);
- return ret < 0 ? ret : req.fd;
-}
/**
* gpiotools_request_line() - request gpio lines in a gpiochip
@@ -215,27 +147,6 @@ int gpiotools_get_values(const int fd, struct
gpio_v2_line_values *values)
return ret;
}
-/**
- * gpiotools_release_linehandle(): Release the line(s) of gpiochip
- * @fd:The fd returned by
- * gpiotools_request_linehandle().
- *
- * Return: On success return 0;
- * On failure return the errno.
- */
-int gpiotools_release_linehandle(const int fd)
-{
- int ret;
-
- ret = close(fd);
- if (ret == -1) {
- perror("Failed to close GPIO LINEHANDLE device file");
- ret = -errno;
- }
-
- return ret;
-}
-
/**
* gpiotools_release_line(): Release the line(s) of gpiochip
* @fd:The fd returned by
diff --git a/tools/gpio/gpio-utils.h b/tools/gpio/gpio-utils.h
index 6c69a9f1c253..8af7c8ee19ce 100644
--- a/tools/gpio/gpio-utils.h
+++ b/tools/gpio/gpio-utils.h
@@ -24,12 +24,6 @@ static inline int check_prefix(const char *str, const char
*prefix)
strncmp(str, prefix, strlen(prefix)) == 0;
}
-int gpiotools_request_linehandle(const char *device_name, unsigned int *lines,
-unsigned int num_lines, unsigned int flag,
-struct gpiohandle_data *data,
-const char *consumer_label);
-int gpiotools_release_line
[PATCH v3 4/7] selftests: remove obsolete gpio references from kselftest_deps.sh
GPIO Makefile has been greatly simplified so remove references to lines
which no longer exist.
Signed-off-by: Kent Gibson
Acked-by: Linus Walleij
Reviewed-by: Bartosz Golaszewski
---
tools/testing/selftests/kselftest_deps.sh | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kselftest_deps.sh
b/tools/testing/selftests/kselftest_deps.sh
index bbc04646346b..00e60d6eb16b 100755
--- a/tools/testing/selftests/kselftest_deps.sh
+++ b/tools/testing/selftests/kselftest_deps.sh
@@ -129,13 +129,11 @@ l2_tests=$(grep -r --include=Makefile ": LDLIBS" | \
grep -v "VAR_LDLIBS" | awk -F: '{print $1}')
# Level 3
-# gpio, memfd and others use pkg-config to find mount and fuse libs
+# memfd and others use pkg-config to find mount and fuse libs
# respectively and save it in VAR_LDLIBS. If pkg-config doesn't find
# any, VAR_LDLIBS set to default.
# Use the default value and filter out pkg-config for dependency check.
# e.g:
-# gpio/Makefile
-# VAR_LDLIBS := $(shell pkg-config --libs mount) 2>/dev/null)
# memfd/Makefile
# VAR_LDLIBS := $(shell pkg-config fuse --libs 2>/dev/null)
--
2.30.0
[PATCH v3 3/7] selftests: remove obsolete build restriction for gpio
Build restrictions related to the gpio-mockup-chardev helper are no longer relevant so remove them. Signed-off-by: Kent Gibson Acked-by: Linus Walleij Reviewed-by: Bartosz Golaszewski --- tools/testing/selftests/Makefile | 9 - 1 file changed, 9 deletions(-) diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index afbab4aeef3c..75a30a5d9945 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -123,15 +123,6 @@ ARCH ?= $(SUBARCH) export KSFT_KHDR_INSTALL_DONE := 1 export BUILD -# build and run gpio when output directory is the src dir. -# gpio has dependency on tools/gpio and builds tools/gpio -# objects in the src directory in all cases making the src -# repo dirty even when objects are relocated. -ifneq (1,$(DEFAULT_INSTALL_HDR_PATH)) - TMP := $(filter-out gpio, $(TARGETS)) - TARGETS := $(TMP) -endif - # set default goal to all, so make without a target runs all, even when # all isn't the first target in the file. .DEFAULT_GOAL := all -- 2.30.0
[PATCH v3 1/7] selftests: gpio: rework and simplify test implementation
The GPIO mockup selftests are overly complicated with separate
implementations of the tests for sysfs and cdev uAPI, and with the cdev
implementation being dependent on tools/gpio and libmount.
Rework the test implementation to provide a common test suite with a
simplified pluggable uAPI interface. The cdev implementation utilises
the GPIO uAPI directly to remove the dependence on tools/gpio.
The simplified uAPI interface removes the need for any file system mount
checks in C, and so removes the dependence on libmount.
The rework also fixes the sysfs test implementation which has been broken
since the device created in the multiple gpiochip case was split into
separate devices.
Fixes: 8a39f597bcfd ("gpio: mockup: rework device probing")
Signed-off-by: Kent Gibson
Acked-by: Linus Walleij
Reviewed-by: Bartosz Golaszewski
---
tools/testing/selftests/gpio/Makefile | 26 +-
.../testing/selftests/gpio/gpio-mockup-cdev.c | 139 +
.../selftests/gpio/gpio-mockup-sysfs.sh | 168 ++
tools/testing/selftests/gpio/gpio-mockup.sh | 490 --
4 files changed, 535 insertions(+), 288 deletions(-)
create mode 100644 tools/testing/selftests/gpio/gpio-mockup-cdev.c
diff --git a/tools/testing/selftests/gpio/Makefile
b/tools/testing/selftests/gpio/Makefile
index 41582fe485ee..39f2bbe8dd3d 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,31 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
-VAR_CFLAGS := $(shell pkg-config --cflags mount 2>/dev/null)
-VAR_LDLIBS := $(shell pkg-config --libs mount 2>/dev/null)
-ifeq ($(VAR_LDLIBS),)
-VAR_LDLIBS := -lmount -I/usr/include/libmount
-endif
-
-CFLAGS += -O2 -g -std=gnu99 -Wall -I../../../../usr/include/ $(VAR_CFLAGS)
-LDLIBS += $(VAR_LDLIBS)
-
TEST_PROGS := gpio-mockup.sh
TEST_FILES := gpio-mockup-sysfs.sh
-TEST_GEN_PROGS_EXTENDED := gpio-mockup-chardev
+TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev
-KSFT_KHDR_INSTALL := 1
include ../lib.mk
-
-GPIODIR := $(realpath ../../../gpio)
-GPIOOUT := $(OUTPUT)/tools-gpio/
-GPIOOBJ := $(GPIOOUT)/gpio-utils.o
-
-CLEAN += ; $(RM) -rf $(GPIOOUT)
-
-$(TEST_GEN_PROGS_EXTENDED): $(GPIOOBJ)
-
-$(GPIOOUT):
- mkdir -p $@
-
-$(GPIOOBJ): $(GPIOOUT)
- $(MAKE) OUTPUT=$(GPIOOUT) -C $(GPIODIR)
diff --git a/tools/testing/selftests/gpio/gpio-mockup-cdev.c
b/tools/testing/selftests/gpio/gpio-mockup-cdev.c
new file mode 100644
index ..3a7fc3ac1349
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-mockup-cdev.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * GPIO mockup cdev test helper
+ *
+ * Copyright (C) 2020 Kent Gibson
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define CONSUMER "gpio-mockup-cdev"
+
+static int request_line_v1(int cfd, unsigned int offset,
+ uint32_t flags, unsigned int val)
+{
+ struct gpiohandle_request req;
+ int ret;
+
+ memset(, 0, sizeof(req));
+ req.lines = 1;
+ req.lineoffsets[0] = offset;
+ req.flags = flags;
+ strcpy(req.consumer_label, CONSUMER);
+ if (flags & GPIOHANDLE_REQUEST_OUTPUT)
+ req.default_values[0] = val;
+
+ ret = ioctl(cfd, GPIO_GET_LINEHANDLE_IOCTL, );
+ if (ret == -1)
+ return -errno;
+ return req.fd;
+}
+
+static int get_value_v1(int lfd)
+{
+ struct gpiohandle_data vals;
+ int ret;
+
+ memset(, 0, sizeof(vals));
+ ret = ioctl(lfd, GPIOHANDLE_GET_LINE_VALUES_IOCTL, );
+ if (ret == -1)
+ return -errno;
+ return vals.values[0];
+}
+
+static void usage(char *prog)
+{
+ printf("Usage: %s [-l] [-b ] [-s ] [-u ]
\n", prog);
+ printf("-b: set line bias to one of pull-down, pull-up,
disabled\n");
+ printf(" (default is to leave bias unchanged):\n");
+ printf("-l: set line active low (default is active high)\n");
+ printf("-s: set line value (default is to get line value)\n");
+ exit(-1);
+}
+
+static int wait_signal(void)
+{
+ int sig;
+ sigset_t wset;
+
+ sigemptyset();
+ sigaddset(, SIGHUP);
+ sigaddset(, SIGINT);
+ sigaddset(, SIGTERM);
+ sigwait(, );
+
+ return sig;
+}
+
+int main(int argc, char *argv[])
+{
+ char *chip;
+ int opt, ret, cfd, lfd;
+ unsigned int offset, val;
+ uint32_t flags_v1;
+
+ ret = 0;
+ flags_v1 = GPIOHANDLE_REQUEST_INPUT;
+
+ while ((opt = getopt(argc, argv, "lb:s:u:")) != -1) {
+ switch (opt) {
+ case 'l':
+ flags_v1 |= GPIOHANDLE_REQUEST_ACTIVE_LOW;
+ break;
+ case 'b':
+ if (strcmp("pull-up", optarg) == 0)
+
[PATCH v3 2/7] selftests: gpio: remove obsolete gpio-mockup-chardev.c
GPIO selftests have changed to new gpio-mockup-cdev helper, so remove
old gpio-mockup-chardev helper.
Signed-off-by: Kent Gibson
Acked-by: Linus Walleij
Reviewed-by: Bartosz Golaszewski
---
.../selftests/gpio/gpio-mockup-chardev.c | 323 --
1 file changed, 323 deletions(-)
delete mode 100644 tools/testing/selftests/gpio/gpio-mockup-chardev.c
diff --git a/tools/testing/selftests/gpio/gpio-mockup-chardev.c
b/tools/testing/selftests/gpio/gpio-mockup-chardev.c
deleted file mode 100644
index 73ead8828d3a..
--- a/tools/testing/selftests/gpio/gpio-mockup-chardev.c
+++ /dev/null
@@ -1,323 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * GPIO chardev test helper
- *
- * Copyright (C) 2016 Bamvor Jian Zhang
- */
-
-#define _GNU_SOURCE
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include "../../../gpio/gpio-utils.h"
-
-#define CONSUMER "gpio-selftest"
-#defineGC_NUM 10
-enum direction {
- OUT,
- IN
-};
-
-static int get_debugfs(char **path)
-{
- struct libmnt_context *cxt;
- struct libmnt_table *tb;
- struct libmnt_iter *itr = NULL;
- struct libmnt_fs *fs;
- int found = 0, ret;
-
- cxt = mnt_new_context();
- if (!cxt)
- err(EXIT_FAILURE, "libmount context allocation failed");
-
- itr = mnt_new_iter(MNT_ITER_FORWARD);
- if (!itr)
- err(EXIT_FAILURE, "failed to initialize libmount iterator");
-
- if (mnt_context_get_mtab(cxt, ))
- err(EXIT_FAILURE, "failed to read mtab");
-
- while (mnt_table_next_fs(tb, itr, ) == 0) {
- const char *type = mnt_fs_get_fstype(fs);
-
- if (!strcmp(type, "debugfs")) {
- found = 1;
- break;
- }
- }
- if (found) {
- ret = asprintf(path, "%s/gpio", mnt_fs_get_target(fs));
- if (ret < 0)
- err(EXIT_FAILURE, "failed to format string");
- }
-
- mnt_free_iter(itr);
- mnt_free_context(cxt);
-
- if (!found)
- return -1;
-
- return 0;
-}
-
-static int gpio_debugfs_get(const char *consumer, int *dir, int *value)
-{
- char *debugfs;
- FILE *f;
- char *line = NULL;
- size_t len = 0;
- char *cur;
- int found = 0;
-
- if (get_debugfs() != 0)
- err(EXIT_FAILURE, "debugfs is not mounted");
-
- f = fopen(debugfs, "r");
- if (!f)
- err(EXIT_FAILURE, "read from gpio debugfs failed");
-
- /*
-* gpio-2 (|gpio-selftest ) in lo
-*/
- while (getline(, , f) != -1) {
- cur = strstr(line, consumer);
- if (cur == NULL)
- continue;
-
- cur = strchr(line, ')');
- if (!cur)
- continue;
-
- cur += 2;
- if (!strncmp(cur, "out", 3)) {
- *dir = OUT;
- cur += 4;
- } else if (!strncmp(cur, "in", 2)) {
- *dir = IN;
- cur += 4;
- }
-
- if (!strncmp(cur, "hi", 2))
- *value = 1;
- else if (!strncmp(cur, "lo", 2))
- *value = 0;
-
- found = 1;
- break;
- }
- free(debugfs);
- fclose(f);
- free(line);
-
- if (!found)
- return -1;
-
- return 0;
-}
-
-static struct gpiochip_info *list_gpiochip(const char *gpiochip_name, int *ret)
-{
- struct gpiochip_info *cinfo;
- struct gpiochip_info *current;
- const struct dirent *ent;
- DIR *dp;
- char *chrdev_name;
- int fd;
- int i = 0;
-
- cinfo = calloc(sizeof(struct gpiochip_info) * 4, GC_NUM + 1);
- if (!cinfo)
- err(EXIT_FAILURE, "gpiochip_info allocation failed");
-
- current = cinfo;
- dp = opendir("/dev");
- if (!dp) {
- *ret = -errno;
- goto error_out;
- } else {
- *ret = 0;
- }
-
- while (ent = readdir(dp), ent) {
- if (check_prefix(ent->d_name, "gpiochip")) {
- *ret = asprintf(_name, "/dev/%s", ent->d_name);
- if (*ret < 0)
- goto error_out;
-
- fd = open(chrdev_name, 0);
- if (fd == -1) {
- *ret = -errno;
- fprintf(stderr, "Failed to open %s\n&qu
[PATCH v3 0/7] selftests: gpio: rework and port to GPIO uAPI v2
Initially I just wanted to port the selftests to the latest GPIO uAPI, but on finding that, due to dependency issues, the selftests are not built for the buildroot environments that I do most of my GPIO testing in, I decided to take a closer look. The first patch is essentially a rewrite of the exising test suite. It uses a simplified abstraction of the uAPI interfaces to allow a common test suite to test the gpio-mockup using either of the uAPI interfaces. The simplified cdev interface is implemented in gpio-mockup.sh, with the actual driving of the uAPI implemented in gpio-mockup-cdev.c. The simplified sysfs interface replaces gpio-mockup-sysfs.sh and is loaded over the cdev implementation when selected. The new tests should also be simpler to extend to cover new mockup interfaces, such as the one Bart has been working on. I have dropped support for testing modules other than gpio-mockup from the command line options, as the tests are very gpio-mockup specific so I didn't see any calling for it. I have also tried to emphasise in the test output that the tests are covering the gpio-mockup itself. They do perform some implicit testing of gpiolib and the uAPI interfaces, and so can be useful as smoke tests for those, but their primary focus is the gpio-mockup. Patches 2 through 5 do some cleaning up that is now possible with the new implementation, including enabling building in buildroot environments. Patch 4 doesn't strictly clean up all the old gpio references that it could - the gpio was the only Level 1 test, so the Level 1 tests could potentially be removed, but I was unsure if there may be other implications to removing a whole test level, or that it may be useful as a placeholder in case other static LDLIBS tests are added in the future?? Patch 6 finally gets around to porting the tests to the latest GPIO uAPI. And Patch 7 updates the config to set the CONFIG_GPIO_CDEV option that was added in v5.10. Cheers, Kent. Changes v2 -> v3: - remove 'commit' from Fixes tag in patch 1. - rebase on Bart's gpio/for-next Changes v1 -> v2 (all in patch 1 and gpio-mockup.sh unless stated otherwise): - reorder includes in gpio-mockup-cdev.c - a multitude of improvements to gpio-mockup.sh and gpio-mockup-sysfs.sh based on Andy's review comments - improved cleanup to ensure all child processes are killed on exit - added race condition prevention or mitigation including the wait in release_line, the retries in assert_mock, the assert_mock in set_mock, and the sleep in set_line Kent Gibson (7): selftests: gpio: rework and simplify test implementation selftests: gpio: remove obsolete gpio-mockup-chardev.c selftests: remove obsolete build restriction for gpio selftests: remove obsolete gpio references from kselftest_deps.sh tools: gpio: remove uAPI v1 code no longer used by selftests selftests: gpio: port to GPIO uAPI v2 selftests: gpio: add CONFIG_GPIO_CDEV to config tools/gpio/gpio-utils.c | 89 tools/gpio/gpio-utils.h | 6 - tools/testing/selftests/Makefile | 9 - tools/testing/selftests/gpio/Makefile | 26 +- tools/testing/selftests/gpio/config | 1 + .../testing/selftests/gpio/gpio-mockup-cdev.c | 198 +++ .../selftests/gpio/gpio-mockup-chardev.c | 323 .../selftests/gpio/gpio-mockup-sysfs.sh | 168 ++ tools/testing/selftests/gpio/gpio-mockup.sh | 497 -- tools/testing/selftests/kselftest_deps.sh | 4 +- 10 files changed, 603 insertions(+), 718 deletions(-) create mode 100644 tools/testing/selftests/gpio/gpio-mockup-cdev.c delete mode 100644 tools/testing/selftests/gpio/gpio-mockup-chardev.c base-commit: 64e6066e16b8c562983dd9d33e604c0001ae0fc7 -- 2.30.0
Re: [PATCH v2 0/7] selftests: gpio: rework and port to GPIO uAPI v2
On Tue, Jan 19, 2021 at 12:02:13PM +0100, Bartosz Golaszewski wrote: > On Tue, Jan 19, 2021 at 11:59 AM Kent Gibson wrote: > > > > On Tue, Jan 19, 2021 at 11:37:46AM +0100, Bartosz Golaszewski wrote: > > > On Tue, Jan 19, 2021 at 1:35 AM Kent Gibson wrote: > > > > > > > > On Mon, Jan 18, 2021 at 04:04:51PM +0100, Linus Walleij wrote: > > > > > On Thu, Jan 7, 2021 at 3:58 AM Kent Gibson > > > > > wrote: > > > > > > > > > > > selftests: gpio: rework and simplify test implementation > > > > > > selftests: gpio: remove obsolete gpio-mockup-chardev.c > > > > > > selftests: remove obsolete build restriction for gpio > > > > > > selftests: remove obsolete gpio references from kselftest_deps.sh > > > > > > tools: gpio: remove uAPI v1 code no longer used by selftests > > > > > > selftests: gpio: port to GPIO uAPI v2 > > > > > > selftests: gpio: add CONFIG_GPIO_CDEV to config > > > > > > > > > > Bartosz I think you can just merge these patches into the GPIO tree, > > > > > at least > > > > > I think that is what I have done in the past. > > > > > > > > > > > > > Could you touch up that Fixes tag in patch 1 if you merge v2? > > > > > > > > Thanks, > > > > Kent. > > > > > > Kent, > > > > > > This doesn't apply to my for-next branch - there's a conflict in > > > tools/testing/selftests/gpio/Makefile, could you take a look? > > > > > > > Which is your for-next branch? > > > > The patch set is based on and applies cleanly to gpio/for-next 7ac554888233, > > so I'm not sure which branch you are targetting. > > > > Cheers, > > Kent. > > Linus W is not picking up patches this release - everything goes through: > > git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next > > Sorry for the confusion. > Ah, ok Michael Ellerman has been improving the Makefile toworkaround the build dependency issues for some cases. My changes supersede all that - a basic Makefile is now sufficient. I'll send out a v3 shortly. Cheers, Kent.
Re: [PATCH v2 0/7] selftests: gpio: rework and port to GPIO uAPI v2
On Tue, Jan 19, 2021 at 11:37:46AM +0100, Bartosz Golaszewski wrote: > On Tue, Jan 19, 2021 at 1:35 AM Kent Gibson wrote: > > > > On Mon, Jan 18, 2021 at 04:04:51PM +0100, Linus Walleij wrote: > > > On Thu, Jan 7, 2021 at 3:58 AM Kent Gibson wrote: > > > > > > > selftests: gpio: rework and simplify test implementation > > > > selftests: gpio: remove obsolete gpio-mockup-chardev.c > > > > selftests: remove obsolete build restriction for gpio > > > > selftests: remove obsolete gpio references from kselftest_deps.sh > > > > tools: gpio: remove uAPI v1 code no longer used by selftests > > > > selftests: gpio: port to GPIO uAPI v2 > > > > selftests: gpio: add CONFIG_GPIO_CDEV to config > > > > > > Bartosz I think you can just merge these patches into the GPIO tree, at > > > least > > > I think that is what I have done in the past. > > > > > > > Could you touch up that Fixes tag in patch 1 if you merge v2? > > > > Thanks, > > Kent. > > Kent, > > This doesn't apply to my for-next branch - there's a conflict in > tools/testing/selftests/gpio/Makefile, could you take a look? > Which is your for-next branch? The patch set is based on and applies cleanly to gpio/for-next 7ac554888233, so I'm not sure which branch you are targetting. Cheers, Kent.
Re: [PATCH v2 0/7] selftests: gpio: rework and port to GPIO uAPI v2
On Mon, Jan 18, 2021 at 04:04:51PM +0100, Linus Walleij wrote: > On Thu, Jan 7, 2021 at 3:58 AM Kent Gibson wrote: > > > selftests: gpio: rework and simplify test implementation > > selftests: gpio: remove obsolete gpio-mockup-chardev.c > > selftests: remove obsolete build restriction for gpio > > selftests: remove obsolete gpio references from kselftest_deps.sh > > tools: gpio: remove uAPI v1 code no longer used by selftests > > selftests: gpio: port to GPIO uAPI v2 > > selftests: gpio: add CONFIG_GPIO_CDEV to config > > Bartosz I think you can just merge these patches into the GPIO tree, at least > I think that is what I have done in the past. > Could you touch up that Fixes tag in patch 1 if you merge v2? Thanks, Kent.
Re: [PATCH v2 1/7] selftests: gpio: rework and simplify test implementation
On Thu, Jan 07, 2021 at 10:57:25AM +0800, Kent Gibson wrote:
> The GPIO mockup selftests are overly complicated with separate
> implementations of the tests for sysfs and cdev uAPI, and with the cdev
> implementation being dependent on tools/gpio and libmount.
>
> Rework the test implementation to provide a common test suite with a
> simplified pluggable uAPI interface. The cdev implementation utilises
> the GPIO uAPI directly to remove the dependence on tools/gpio.
> The simplified uAPI interface removes the need for any file system mount
> checks in C, and so removes the dependence on libmount.
>
> The rework also fixes the sysfs test implementation which has been broken
> since the device created in the multiple gpiochip case was split into
> separate devices.
>
> Fixes: commit 8a39f597bcfd ("gpio: mockup: rework device probing")
> Signed-off-by: Kent Gibson
> Acked-by: Linus Walleij
> ---
Just a note that the 'commit' should be removed from the Fixes tag.
That will be fixed in v3.
The patches have been reviewed from the gpio side - any feedback from the
selftest side?
Cheers,
Kent.
Re: linux-next: Fixes tags need some work in the gpio-brgl-fixes tree
On Mon, Jan 18, 2021 at 08:21:09AM +1100, Stephen Rothwell wrote:
> Hi all,
>
> In commit
>
> ec6c3364b816 ("tools: gpio: fix %llu warning in gpio-watch.c")
>
> Fixes tag
>
> Fixes: commit 33f0c47b8fb4 ("tools: gpio: implement gpio-watch")
>
> has these problem(s):
>
> - leading word 'commit' unexpected
>
> In commit
>
> 56835f1c14bc ("tools: gpio: fix %llu warning in gpio-event-mon.c")
>
> Fixes tag
>
> Fixes: commit 03fd11b03362 ("tools/gpio/gpio-event-mon: fix warning")
>
> has these problem(s):
>
> - leading word 'commit' unexpected
>
Damn - not sure why I started doing that - it is in one of my selftest
patches as well, and checkpatches.pl doesn't pick it up :-(.
Cheers,
Kent.
[PATCH 2/2] tools: gpio: fix %llu warning in gpio-watch.c
Some platforms, such as mips64, don't map __u64 to long long unsigned
int so using %llu produces a warning:
gpio-watch.c: In function ‘main’:
gpio-watch.c:89:30: warning: format ‘%llu’ expects argument of type ‘long long
unsigned int’, but argument 4 has type ‘__u64’ {aka ‘long unsigned int’}
[-Wformat=]
89 |printf("line %u: %s at %llu\n",
| ~~~^
| |
| long long unsigned int
| %lu
90 | chg.info.offset, event, chg.timestamp_ns);
|
| |
| __u64 {aka long unsigned int}
Replace the %llu with PRIu64 and cast the argument to uint64_t.
Fixes: commit 33f0c47b8fb4 ("tools: gpio: implement gpio-watch")
Signed-off-by: Kent Gibson
---
tools/gpio/gpio-watch.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/gpio/gpio-watch.c b/tools/gpio/gpio-watch.c
index f229ec62301b..41e76d244192 100644
--- a/tools/gpio/gpio-watch.c
+++ b/tools/gpio/gpio-watch.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -86,8 +87,8 @@ int main(int argc, char **argv)
return EXIT_FAILURE;
}
- printf("line %u: %s at %llu\n",
- chg.info.offset, event, chg.timestamp_ns);
+ printf("line %u: %s at %" PRIu64 "\n",
+ chg.info.offset, event,
(uint64_t)chg.timestamp_ns);
}
}
--
2.30.0
[PATCH 1/2] tools: gpio: fix %llu warning in gpio-event-mon.c
Some platforms, such as mips64, don't map __u64 to long long unsigned
int so using %llu produces a warning:
gpio-event-mon.c:110:37: warning: format ‘%llu’ expects argument of type ‘long
long unsigned int’, but argument 3 has type ‘__u64’ {aka ‘long unsigned int’}
[-Wformat=]
110 | fprintf(stdout, "GPIO EVENT at %llu on line %d (%d|%d) ",
| ~~~^
| |
| long long unsigned int
| %lu
111 |event.timestamp_ns, event.offset, event.line_seqno,
|~~
| |
| __u64 {aka long unsigned int}
Replace the %llu with PRIu64 and cast the argument to uint64_t.
Fixes: commit 03fd11b03362 ("tools/gpio/gpio-event-mon: fix warning")
Signed-off-by: Kent Gibson
---
tools/gpio/gpio-event-mon.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/gpio/gpio-event-mon.c b/tools/gpio/gpio-event-mon.c
index cacd66ad7926..a2b233fdb572 100644
--- a/tools/gpio/gpio-event-mon.c
+++ b/tools/gpio/gpio-event-mon.c
@@ -107,8 +107,8 @@ int monitor_device(const char *device_name,
ret = -EIO;
break;
}
- fprintf(stdout, "GPIO EVENT at %llu on line %d (%d|%d) ",
- event.timestamp_ns, event.offset, event.line_seqno,
+ fprintf(stdout, "GPIO EVENT at %" PRIu64 " on line %d (%d|%d) ",
+ (uint64_t)event.timestamp_ns, event.offset,
event.line_seqno,
event.seqno);
switch (event.id) {
case GPIO_V2_LINE_EVENT_RISING_EDGE:
--
2.30.0
[PATCH 0/2] tools: gpio: fix %llu warnings
Fix a couple of warnings that I ran across while testing selftest changes. Sorry about the repetition in the checkin comments, but as the problem was introduced to the two files separately it seemed more appropriate than tying their history together. Cheers, Kent. Kent Gibson (2): tools: gpio: fix %llu warning in gpio-event-mon.c tools: gpio: fix %llu warning in gpio-watch.c tools/gpio/gpio-event-mon.c | 4 ++-- tools/gpio/gpio-watch.c | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) -- 2.30.0
[PATCH v2 4/7] selftests: remove obsolete gpio references from kselftest_deps.sh
GPIO Makefile has been greatly simplified so remove references to lines
which no longer exist.
Signed-off-by: Kent Gibson
Acked-by: Linus Walleij
---
tools/testing/selftests/kselftest_deps.sh | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kselftest_deps.sh
b/tools/testing/selftests/kselftest_deps.sh
index bbc04646346b..00e60d6eb16b 100755
--- a/tools/testing/selftests/kselftest_deps.sh
+++ b/tools/testing/selftests/kselftest_deps.sh
@@ -129,13 +129,11 @@ l2_tests=$(grep -r --include=Makefile ": LDLIBS" | \
grep -v "VAR_LDLIBS" | awk -F: '{print $1}')
# Level 3
-# gpio, memfd and others use pkg-config to find mount and fuse libs
+# memfd and others use pkg-config to find mount and fuse libs
# respectively and save it in VAR_LDLIBS. If pkg-config doesn't find
# any, VAR_LDLIBS set to default.
# Use the default value and filter out pkg-config for dependency check.
# e.g:
-# gpio/Makefile
-# VAR_LDLIBS := $(shell pkg-config --libs mount) 2>/dev/null)
# memfd/Makefile
# VAR_LDLIBS := $(shell pkg-config fuse --libs 2>/dev/null)
--
2.30.0
[PATCH v2 6/7] selftests: gpio: port to GPIO uAPI v2
Add a port to the GPIO uAPI v2 interface and make it the default.
Signed-off-by: Kent Gibson
Acked-by: Linus Walleij
---
.../testing/selftests/gpio/gpio-mockup-cdev.c | 75 +--
tools/testing/selftests/gpio/gpio-mockup.sh | 11 ++-
2 files changed, 76 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/gpio/gpio-mockup-cdev.c
b/tools/testing/selftests/gpio/gpio-mockup-cdev.c
index 3a7fc3ac1349..e83eac71621a 100644
--- a/tools/testing/selftests/gpio/gpio-mockup-cdev.c
+++ b/tools/testing/selftests/gpio/gpio-mockup-cdev.c
@@ -18,6 +18,44 @@
#define CONSUMER "gpio-mockup-cdev"
+static int request_line_v2(int cfd, unsigned int offset,
+ uint64_t flags, unsigned int val)
+{
+ struct gpio_v2_line_request req;
+ int ret;
+
+ memset(, 0, sizeof(req));
+ req.num_lines = 1;
+ req.offsets[0] = offset;
+ req.config.flags = flags;
+ strcpy(req.consumer, CONSUMER);
+ if (flags & GPIO_V2_LINE_FLAG_OUTPUT) {
+ req.config.num_attrs = 1;
+ req.config.attrs[0].mask = 1;
+ req.config.attrs[0].attr.id =
GPIO_V2_LINE_ATTR_ID_OUTPUT_VALUES;
+ if (val)
+ req.config.attrs[0].attr.values = 1;
+ }
+ ret = ioctl(cfd, GPIO_V2_GET_LINE_IOCTL, );
+ if (ret == -1)
+ return -errno;
+ return req.fd;
+}
+
+
+static int get_value_v2(int lfd)
+{
+ struct gpio_v2_line_values vals;
+ int ret;
+
+ memset(, 0, sizeof(vals));
+ vals.mask = 1;
+ ret = ioctl(lfd, GPIO_V2_LINE_GET_VALUES_IOCTL, );
+ if (ret == -1)
+ return -errno;
+ return vals.bits & 0x1;
+}
+
static int request_line_v1(int cfd, unsigned int offset,
uint32_t flags, unsigned int val)
{
@@ -57,6 +95,7 @@ static void usage(char *prog)
printf(" (default is to leave bias unchanged):\n");
printf("-l: set line active low (default is active high)\n");
printf("-s: set line value (default is to get line value)\n");
+ printf("-u: uAPI version to use (default is 2)\n");
exit(-1);
}
@@ -78,29 +117,42 @@ int main(int argc, char *argv[])
{
char *chip;
int opt, ret, cfd, lfd;
- unsigned int offset, val;
+ unsigned int offset, val, abiv;
uint32_t flags_v1;
+ uint64_t flags_v2;
+ abiv = 2;
ret = 0;
flags_v1 = GPIOHANDLE_REQUEST_INPUT;
+ flags_v2 = GPIO_V2_LINE_FLAG_INPUT;
while ((opt = getopt(argc, argv, "lb:s:u:")) != -1) {
switch (opt) {
case 'l':
flags_v1 |= GPIOHANDLE_REQUEST_ACTIVE_LOW;
+ flags_v2 |= GPIO_V2_LINE_FLAG_ACTIVE_LOW;
break;
case 'b':
- if (strcmp("pull-up", optarg) == 0)
+ if (strcmp("pull-up", optarg) == 0) {
flags_v1 |= GPIOHANDLE_REQUEST_BIAS_PULL_UP;
- else if (strcmp("pull-down", optarg) == 0)
+ flags_v2 |= GPIO_V2_LINE_FLAG_BIAS_PULL_UP;
+ } else if (strcmp("pull-down", optarg) == 0) {
flags_v1 |= GPIOHANDLE_REQUEST_BIAS_PULL_DOWN;
- else if (strcmp("disabled", optarg) == 0)
+ flags_v2 |= GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN;
+ } else if (strcmp("disabled", optarg) == 0) {
flags_v1 |= GPIOHANDLE_REQUEST_BIAS_DISABLE;
+ flags_v2 |= GPIO_V2_LINE_FLAG_BIAS_DISABLED;
+ }
break;
case 's':
val = atoi(optarg);
flags_v1 &= ~GPIOHANDLE_REQUEST_INPUT;
flags_v1 |= GPIOHANDLE_REQUEST_OUTPUT;
+ flags_v2 &= ~GPIO_V2_LINE_FLAG_INPUT;
+ flags_v2 |= GPIO_V2_LINE_FLAG_OUTPUT;
+ break;
+ case 'u':
+ abiv = atoi(optarg);
break;
default:
usage(argv[0]);
@@ -119,7 +171,10 @@ int main(int argc, char *argv[])
return -errno;
}
- lfd = request_line_v1(cfd, offset, flags_v1, val);
+ if (abiv == 1)
+ lfd = request_line_v1(cfd, offset, flags_v1, val);
+ else
+ lfd = request_line_v2(cfd, offset, flags_v2, val);
close(cfd);
@@ -128,10 +183,14 @@ int main(int argc, char *argv[])
return lfd;
}
- if (flags_v1 & GPIOHANDLE_REQUEST_OUTPUT)
+ if
[PATCH v2 3/7] selftests: remove obsolete build restriction for gpio
Build restrictions related to the gpio-mockup-chardev helper are no longer relevant so remove them. Signed-off-by: Kent Gibson Acked-by: Linus Walleij --- tools/testing/selftests/Makefile | 9 - 1 file changed, 9 deletions(-) diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index d9c283503159..5411041e63a0 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -121,15 +121,6 @@ ARCH ?= $(SUBARCH) export KSFT_KHDR_INSTALL_DONE := 1 export BUILD -# build and run gpio when output directory is the src dir. -# gpio has dependency on tools/gpio and builds tools/gpio -# objects in the src directory in all cases making the src -# repo dirty even when objects are relocated. -ifneq (1,$(DEFAULT_INSTALL_HDR_PATH)) - TMP := $(filter-out gpio, $(TARGETS)) - TARGETS := $(TMP) -endif - # set default goal to all, so make without a target runs all, even when # all isn't the first target in the file. .DEFAULT_GOAL := all -- 2.30.0
[PATCH v2 1/7] selftests: gpio: rework and simplify test implementation
The GPIO mockup selftests are overly complicated with separate
implementations of the tests for sysfs and cdev uAPI, and with the cdev
implementation being dependent on tools/gpio and libmount.
Rework the test implementation to provide a common test suite with a
simplified pluggable uAPI interface. The cdev implementation utilises
the GPIO uAPI directly to remove the dependence on tools/gpio.
The simplified uAPI interface removes the need for any file system mount
checks in C, and so removes the dependence on libmount.
The rework also fixes the sysfs test implementation which has been broken
since the device created in the multiple gpiochip case was split into
separate devices.
Fixes: commit 8a39f597bcfd ("gpio: mockup: rework device probing")
Signed-off-by: Kent Gibson
Acked-by: Linus Walleij
---
tools/testing/selftests/gpio/Makefile | 26 +-
.../testing/selftests/gpio/gpio-mockup-cdev.c | 139 +
.../selftests/gpio/gpio-mockup-sysfs.sh | 168 ++
tools/testing/selftests/gpio/gpio-mockup.sh | 490 --
4 files changed, 535 insertions(+), 288 deletions(-)
create mode 100644 tools/testing/selftests/gpio/gpio-mockup-cdev.c
diff --git a/tools/testing/selftests/gpio/Makefile
b/tools/testing/selftests/gpio/Makefile
index 32bdc978a711..e4363c64d40d 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,32 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
-VAR_CFLAGS := $(shell pkg-config --cflags mount 2>/dev/null)
-VAR_LDLIBS := $(shell pkg-config --libs mount 2>/dev/null)
-ifeq ($(VAR_LDLIBS),)
-VAR_LDLIBS := -lmount -I/usr/include/libmount
-endif
-
-CFLAGS += -O2 -g -std=gnu99 -Wall -I../../../../usr/include/ $(VAR_CFLAGS)
-LDLIBS += $(VAR_LDLIBS)
-
TEST_PROGS := gpio-mockup.sh
TEST_FILES := gpio-mockup-sysfs.sh
-TEST_PROGS_EXTENDED := gpio-mockup-chardev
-
-GPIODIR := $(realpath ../../../gpio)
-GPIOOBJ := gpio-utils.o
+TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev
-all: $(TEST_PROGS_EXTENDED)
-
-override define CLEAN
- $(RM) $(TEST_PROGS_EXTENDED)
- $(MAKE) -C $(GPIODIR) OUTPUT=$(GPIODIR)/ clean
-endef
-
-KSFT_KHDR_INSTALL := 1
include ../lib.mk
-$(TEST_PROGS_EXTENDED): $(GPIODIR)/$(GPIOOBJ)
-
-$(GPIODIR)/$(GPIOOBJ):
- $(MAKE) OUTPUT=$(GPIODIR)/ -C $(GPIODIR)
diff --git a/tools/testing/selftests/gpio/gpio-mockup-cdev.c
b/tools/testing/selftests/gpio/gpio-mockup-cdev.c
new file mode 100644
index ..3a7fc3ac1349
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-mockup-cdev.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * GPIO mockup cdev test helper
+ *
+ * Copyright (C) 2020 Kent Gibson
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define CONSUMER "gpio-mockup-cdev"
+
+static int request_line_v1(int cfd, unsigned int offset,
+ uint32_t flags, unsigned int val)
+{
+ struct gpiohandle_request req;
+ int ret;
+
+ memset(, 0, sizeof(req));
+ req.lines = 1;
+ req.lineoffsets[0] = offset;
+ req.flags = flags;
+ strcpy(req.consumer_label, CONSUMER);
+ if (flags & GPIOHANDLE_REQUEST_OUTPUT)
+ req.default_values[0] = val;
+
+ ret = ioctl(cfd, GPIO_GET_LINEHANDLE_IOCTL, );
+ if (ret == -1)
+ return -errno;
+ return req.fd;
+}
+
+static int get_value_v1(int lfd)
+{
+ struct gpiohandle_data vals;
+ int ret;
+
+ memset(, 0, sizeof(vals));
+ ret = ioctl(lfd, GPIOHANDLE_GET_LINE_VALUES_IOCTL, );
+ if (ret == -1)
+ return -errno;
+ return vals.values[0];
+}
+
+static void usage(char *prog)
+{
+ printf("Usage: %s [-l] [-b ] [-s ] [-u ]
\n", prog);
+ printf("-b: set line bias to one of pull-down, pull-up,
disabled\n");
+ printf(" (default is to leave bias unchanged):\n");
+ printf("-l: set line active low (default is active high)\n");
+ printf("-s: set line value (default is to get line value)\n");
+ exit(-1);
+}
+
+static int wait_signal(void)
+{
+ int sig;
+ sigset_t wset;
+
+ sigemptyset();
+ sigaddset(, SIGHUP);
+ sigaddset(, SIGINT);
+ sigaddset(, SIGTERM);
+ sigwait(, );
+
+ return sig;
+}
+
+int main(int argc, char *argv[])
+{
+ char *chip;
+ int opt, ret, cfd, lfd;
+ unsigned int offset, val;
+ uint32_t flags_v1;
+
+ ret = 0;
+ flags_v1 = GPIOHANDLE_REQUEST_INPUT;
+
+ while ((opt = getopt(argc, argv, "lb:s:u:")) != -1) {
+ switch (opt) {
+ case 'l':
+ flags_v1 |= GPIOHANDLE_REQUEST_ACTIVE_LOW;
+ break;
+ case 'b':
+ if (strcmp("pull-up", optarg) == 0)
+
[PATCH v2 2/7] selftests: gpio: remove obsolete gpio-mockup-chardev.c
GPIO selftests have changed to new gpio-mockup-cdev helper, so remove
old gpio-mockup-chardev helper.
Signed-off-by: Kent Gibson
Acked-by: Linus Walleij
---
.../selftests/gpio/gpio-mockup-chardev.c | 323 --
1 file changed, 323 deletions(-)
delete mode 100644 tools/testing/selftests/gpio/gpio-mockup-chardev.c
diff --git a/tools/testing/selftests/gpio/gpio-mockup-chardev.c
b/tools/testing/selftests/gpio/gpio-mockup-chardev.c
deleted file mode 100644
index 73ead8828d3a..
--- a/tools/testing/selftests/gpio/gpio-mockup-chardev.c
+++ /dev/null
@@ -1,323 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * GPIO chardev test helper
- *
- * Copyright (C) 2016 Bamvor Jian Zhang
- */
-
-#define _GNU_SOURCE
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include "../../../gpio/gpio-utils.h"
-
-#define CONSUMER "gpio-selftest"
-#defineGC_NUM 10
-enum direction {
- OUT,
- IN
-};
-
-static int get_debugfs(char **path)
-{
- struct libmnt_context *cxt;
- struct libmnt_table *tb;
- struct libmnt_iter *itr = NULL;
- struct libmnt_fs *fs;
- int found = 0, ret;
-
- cxt = mnt_new_context();
- if (!cxt)
- err(EXIT_FAILURE, "libmount context allocation failed");
-
- itr = mnt_new_iter(MNT_ITER_FORWARD);
- if (!itr)
- err(EXIT_FAILURE, "failed to initialize libmount iterator");
-
- if (mnt_context_get_mtab(cxt, ))
- err(EXIT_FAILURE, "failed to read mtab");
-
- while (mnt_table_next_fs(tb, itr, ) == 0) {
- const char *type = mnt_fs_get_fstype(fs);
-
- if (!strcmp(type, "debugfs")) {
- found = 1;
- break;
- }
- }
- if (found) {
- ret = asprintf(path, "%s/gpio", mnt_fs_get_target(fs));
- if (ret < 0)
- err(EXIT_FAILURE, "failed to format string");
- }
-
- mnt_free_iter(itr);
- mnt_free_context(cxt);
-
- if (!found)
- return -1;
-
- return 0;
-}
-
-static int gpio_debugfs_get(const char *consumer, int *dir, int *value)
-{
- char *debugfs;
- FILE *f;
- char *line = NULL;
- size_t len = 0;
- char *cur;
- int found = 0;
-
- if (get_debugfs() != 0)
- err(EXIT_FAILURE, "debugfs is not mounted");
-
- f = fopen(debugfs, "r");
- if (!f)
- err(EXIT_FAILURE, "read from gpio debugfs failed");
-
- /*
-* gpio-2 (|gpio-selftest ) in lo
-*/
- while (getline(, , f) != -1) {
- cur = strstr(line, consumer);
- if (cur == NULL)
- continue;
-
- cur = strchr(line, ')');
- if (!cur)
- continue;
-
- cur += 2;
- if (!strncmp(cur, "out", 3)) {
- *dir = OUT;
- cur += 4;
- } else if (!strncmp(cur, "in", 2)) {
- *dir = IN;
- cur += 4;
- }
-
- if (!strncmp(cur, "hi", 2))
- *value = 1;
- else if (!strncmp(cur, "lo", 2))
- *value = 0;
-
- found = 1;
- break;
- }
- free(debugfs);
- fclose(f);
- free(line);
-
- if (!found)
- return -1;
-
- return 0;
-}
-
-static struct gpiochip_info *list_gpiochip(const char *gpiochip_name, int *ret)
-{
- struct gpiochip_info *cinfo;
- struct gpiochip_info *current;
- const struct dirent *ent;
- DIR *dp;
- char *chrdev_name;
- int fd;
- int i = 0;
-
- cinfo = calloc(sizeof(struct gpiochip_info) * 4, GC_NUM + 1);
- if (!cinfo)
- err(EXIT_FAILURE, "gpiochip_info allocation failed");
-
- current = cinfo;
- dp = opendir("/dev");
- if (!dp) {
- *ret = -errno;
- goto error_out;
- } else {
- *ret = 0;
- }
-
- while (ent = readdir(dp), ent) {
- if (check_prefix(ent->d_name, "gpiochip")) {
- *ret = asprintf(_name, "/dev/%s", ent->d_name);
- if (*ret < 0)
- goto error_out;
-
- fd = open(chrdev_name, 0);
- if (fd == -1) {
- *ret = -errno;
- fprintf(stderr, "Failed to open %s\n",
-
[PATCH v2 5/7] tools: gpio: remove uAPI v1 code no longer used by selftests
gpio-mockup-chardev helper has been obsoleted and removed, so also remove
the tools/gpio code that it, and nothing else, was using.
Signed-off-by: Kent Gibson
Acked-by: Linus Walleij
---
tools/gpio/gpio-utils.c | 89 -
tools/gpio/gpio-utils.h | 6 ---
2 files changed, 95 deletions(-)
diff --git a/tools/gpio/gpio-utils.c b/tools/gpio/gpio-utils.c
index 37187e056c8b..1639b4d832cd 100644
--- a/tools/gpio/gpio-utils.c
+++ b/tools/gpio/gpio-utils.c
@@ -32,74 +32,6 @@
* following api will request gpio lines, do the operation and then
* release these lines.
*/
-/**
- * gpiotools_request_linehandle() - request gpio lines in a gpiochip
- * @device_name: The name of gpiochip without prefix "/dev/",
- * such as "gpiochip0"
- * @lines: An array desired lines, specified by offset
- * index for the associated GPIO device.
- * @num_lines: The number of lines to request.
- * @flag: The new flag for requsted gpio. Reference
- * "linux/gpio.h" for the meaning of flag.
- * @data: Default value will be set to gpio when flag is
- * GPIOHANDLE_REQUEST_OUTPUT.
- * @consumer_label:The name of consumer, such as "sysfs",
- * "powerkey". This is useful for other users to
- * know who is using.
- *
- * Request gpio lines through the ioctl provided by chardev. User
- * could call gpiotools_set_values() and gpiotools_get_values() to
- * read and write respectively through the returned fd. Call
- * gpiotools_release_linehandle() to release these lines after that.
- *
- * Return: On success return the fd;
- * On failure return the errno.
- */
-int gpiotools_request_linehandle(const char *device_name, unsigned int *lines,
-unsigned int num_lines, unsigned int flag,
-struct gpiohandle_data *data,
-const char *consumer_label)
-{
- struct gpiohandle_request req;
- char *chrdev_name;
- int fd;
- int i;
- int ret;
-
- ret = asprintf(_name, "/dev/%s", device_name);
- if (ret < 0)
- return -ENOMEM;
-
- fd = open(chrdev_name, 0);
- if (fd == -1) {
- ret = -errno;
- fprintf(stderr, "Failed to open %s, %s\n",
- chrdev_name, strerror(errno));
- goto exit_free_name;
- }
-
- for (i = 0; i < num_lines; i++)
- req.lineoffsets[i] = lines[i];
-
- req.flags = flag;
- strcpy(req.consumer_label, consumer_label);
- req.lines = num_lines;
- if (flag & GPIOHANDLE_REQUEST_OUTPUT)
- memcpy(req.default_values, data, sizeof(req.default_values));
-
- ret = ioctl(fd, GPIO_GET_LINEHANDLE_IOCTL, );
- if (ret == -1) {
- ret = -errno;
- fprintf(stderr, "Failed to issue %s (%d), %s\n",
- "GPIO_GET_LINEHANDLE_IOCTL", ret, strerror(errno));
- }
-
- if (close(fd) == -1)
- perror("Failed to close GPIO character device file");
-exit_free_name:
- free(chrdev_name);
- return ret < 0 ? ret : req.fd;
-}
/**
* gpiotools_request_line() - request gpio lines in a gpiochip
@@ -215,27 +147,6 @@ int gpiotools_get_values(const int fd, struct
gpio_v2_line_values *values)
return ret;
}
-/**
- * gpiotools_release_linehandle(): Release the line(s) of gpiochip
- * @fd:The fd returned by
- * gpiotools_request_linehandle().
- *
- * Return: On success return 0;
- * On failure return the errno.
- */
-int gpiotools_release_linehandle(const int fd)
-{
- int ret;
-
- ret = close(fd);
- if (ret == -1) {
- perror("Failed to close GPIO LINEHANDLE device file");
- ret = -errno;
- }
-
- return ret;
-}
-
/**
* gpiotools_release_line(): Release the line(s) of gpiochip
* @fd:The fd returned by
diff --git a/tools/gpio/gpio-utils.h b/tools/gpio/gpio-utils.h
index 6c69a9f1c253..8af7c8ee19ce 100644
--- a/tools/gpio/gpio-utils.h
+++ b/tools/gpio/gpio-utils.h
@@ -24,12 +24,6 @@ static inline int check_prefix(const char *str, const char
*prefix)
strncmp(str, prefix, strlen(prefix)) == 0;
}
-int gpiotools_request_linehandle(const char *device_name, unsigned int *lines,
-unsigned int num_lines, unsigned int flag,
-struct gpiohandle_data *data,
-const char *consumer_label);
-int gpiotools_release_linehandle(const int fd);
-
int gpiotools_request_line(const char *device_name,
unsigned int *lines,
unsigned int num_lines,
--
2.30.0
[PATCH v2 7/7] selftests: gpio: add CONFIG_GPIO_CDEV to config
GPIO CDEV is now optional and required for the selftests so add it to the config. Signed-off-by: Kent Gibson Acked-by: Linus Walleij --- tools/testing/selftests/gpio/config | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/gpio/config b/tools/testing/selftests/gpio/config index abaa6902b7b6..ce100342c20b 100644 --- a/tools/testing/selftests/gpio/config +++ b/tools/testing/selftests/gpio/config @@ -1,2 +1,3 @@ CONFIG_GPIOLIB=y +CONFIG_GPIO_CDEV=y CONFIG_GPIO_MOCKUP=m -- 2.30.0
[PATCH v2 0/7] selftests: gpio: rework and port to GPIO uAPI v2
Initially I just wanted to port the selftests to the latest GPIO uAPI, but on finding that, due to dependency issues, the selftests are not built for the buildroot environments that I do most of my GPIO testing in, I decided to take a closer look. The first patch is essentially a rewrite of the exising test suite. It uses a simplified abstraction of the uAPI interfaces to allow a common test suite to test the gpio-mockup using either of the uAPI interfaces. The simplified cdev interface is implemented in gpio-mockup.sh, with the actual driving of the uAPI implemented in gpio-mockup-cdev.c. The simplified sysfs interface replaces gpio-mockup-sysfs.sh and is loaded over the cdev implementation when selected. The new tests should also be simpler to extend to cover new mockup interfaces, such as the one Bart has been working on. I have dropped support for testing modules other than gpio-mockup from the command line options, as the tests are very gpio-mockup specific so I didn't see any calling for it. I have also tried to emphasise in the test output that the tests are covering the gpio-mockup itself. They do perform some implicit testing of gpiolib and the uAPI interfaces, and so can be useful as smoke tests for those, but their primary focus is the gpio-mockup. Patches 2 through 5 do some cleaning up that is now possible with the new implementation, including enabling building in buildroot environments. Patch 4 doesn't strictly clean up all the old gpio references that it could - the gpio was the only Level 1 test, so the Level 1 tests could potentially be removed, but I was unsure if there may be other implications to removing a whole test level, or that it may be useful as a placeholder in case other static LDLIBS tests are added in the future?? Patch 6 finally gets around to porting the tests to the latest GPIO uAPI. And Patch 7 updates the config to set the CONFIG_GPIO_CDEV option that was added in v5.10. Cheers, Kent. Changes v1 -> v2 (all in patch 1 and gpio-mockup.sh unless stated otherwise): - reorder includes in gpio-mockup-cdev.c - a multitude of improvements to gpio-mockup.sh and gpio-mockup-sysfs.sh based on Andy's review comments - improved cleanup to ensure all child processes are killed on exit - added race condition prevention or mitigation including the wait in release_line, the retries in assert_mock, the assert_mock in set_mock, and the sleep in set_line Kent Gibson (7): selftests: gpio: rework and simplify test implementation selftests: gpio: remove obsolete gpio-mockup-chardev.c selftests: remove obsolete build restriction for gpio selftests: remove obsolete gpio references from kselftest_deps.sh tools: gpio: remove uAPI v1 code no longer used by selftests selftests: gpio: port to GPIO uAPI v2 selftests: gpio: add CONFIG_GPIO_CDEV to config tools/gpio/gpio-utils.c | 89 tools/gpio/gpio-utils.h | 6 - tools/testing/selftests/Makefile | 9 - tools/testing/selftests/gpio/Makefile | 26 +- tools/testing/selftests/gpio/config | 1 + .../testing/selftests/gpio/gpio-mockup-cdev.c | 198 +++ .../selftests/gpio/gpio-mockup-chardev.c | 323 .../selftests/gpio/gpio-mockup-sysfs.sh | 168 ++ tools/testing/selftests/gpio/gpio-mockup.sh | 497 -- tools/testing/selftests/kselftest_deps.sh | 4 +- 10 files changed, 603 insertions(+), 718 deletions(-) create mode 100644 tools/testing/selftests/gpio/gpio-mockup-cdev.c delete mode 100644 tools/testing/selftests/gpio/gpio-mockup-chardev.c -- 2.30.0
Re: [PATCH 1/7] selftests: gpio: rework and simplify test implementation
On Mon, Jan 04, 2021 at 11:00:31PM +0800, Kent Gibson wrote: > On Mon, Jan 04, 2021 at 03:52:49PM +0200, Andy Shevchenko wrote: > > On Mon, Jan 4, 2021 at 3:51 AM Kent Gibson wrote: > > > On Sun, Jan 03, 2021 at 05:10:10PM +0200, Andy Shevchenko wrote: > > > > ... > > > > > In this example it is the 508: > > > > > > # e.g. gpiochip0: GPIOs 508-511, parent: platform/gpio-mockup.0, > > > gpio-mockup-A: > > > > > > So I'll use that - unless it is unreliable for some reason? > > > > debugfs is not an ABI and tomorrow this can be changed without notice. > > > > I had a bad feeling that might be the case, and all my current solutions > use debugfs one way or another, so back to the drawing board on that one. > Hang on - the find approach that I was looking at previously only uses /sys/devices/platform, so I'll revert to that one - and add handling for multi-match. Cheers, Kent.
Re: [PATCH 1/7] selftests: gpio: rework and simplify test implementation
On Mon, Jan 04, 2021 at 03:52:49PM +0200, Andy Shevchenko wrote: > On Mon, Jan 4, 2021 at 3:51 AM Kent Gibson wrote: > > On Sun, Jan 03, 2021 at 05:10:10PM +0200, Andy Shevchenko wrote: > > ... > > > In this example it is the 508: > > > > # e.g. gpiochip0: GPIOs 508-511, parent: platform/gpio-mockup.0, > > gpio-mockup-A: > > > > So I'll use that - unless it is unreliable for some reason? > > debugfs is not an ABI and tomorrow this can be changed without notice. > I had a bad feeling that might be the case, and all my current solutions use debugfs one way or another, so back to the drawing board on that one. Thanks, Kent.
Re: [PATCH 1/7] selftests: gpio: rework and simplify test implementation
On Sun, Jan 03, 2021 at 05:10:10PM +0200, Andy Shevchenko wrote:
> On Sun, Jan 3, 2021 at 4:17 AM Kent Gibson wrote:
> > On Sun, Jan 03, 2021 at 12:20:26AM +0200, Andy Shevchenko wrote:
> > > On Sat, Jan 2, 2021 at 4:32 AM Kent Gibson wrote:
>
[snip]
>
> ...
>
> > > > + local platform=`cat $SYSFS/kernel/debug/gpio | grep "$chip:" |
> > > > tr -d ',' | awk '{print $5}'`
> > >
> > > Besides useless use of cat (and tr + awk can be simplified) why are
> >
> > What do you suggest for the tr/awk simplification?
>
> You have `awk`, you can easily switch the entire pipeline to a little
> awk scriptlet.
>
Baah, the number that I'm after is in the $SYSFS/kernel/debug/gpio that I
was pulling the platform from, so I can just pull it directly from there.
No need to go hunting through the file system for the base file - the
range of GPIOs assigned to the chip is right there.
In this example it is the 508:
# e.g. gpiochip0: GPIOs 508-511, parent: platform/gpio-mockup.0, gpio-mockup-A:
So I'll use that - unless it is unreliable for some reason?
Cheers,
Kent.
Re: [PATCH 1/7] selftests: gpio: rework and simplify test implementation
On Sun, Jan 03, 2021 at 05:10:10PM +0200, Andy Shevchenko wrote:
> On Sun, Jan 3, 2021 at 4:17 AM Kent Gibson wrote:
> > On Sun, Jan 03, 2021 at 12:20:26AM +0200, Andy Shevchenko wrote:
> > > On Sat, Jan 2, 2021 at 4:32 AM Kent Gibson wrote:
>
> ...
>
> > > > +#include
> > >
> > > Perhaps include it after system headers?
> >
> > hehe, I blindly sorted them.
> > Should it matter?
>
> I would include more particular headers later.
> Btw system headers can not always be in order because of dependencies.
>
> ...
>
> > > > + local platform=`cat $SYSFS/kernel/debug/gpio | grep "$chip:" |
> > > > tr -d ',' | awk '{print $5}'`
> > >
> > > Besides useless use of cat (and tr + awk can be simplified) why are
> >
> > What do you suggest for the tr/awk simplification?
>
> You have `awk`, you can easily switch the entire pipeline to a little
> awk scriptlet.
>
Ah ok - I was actually going the other way to do away with the awk, so
had replaced it with a pair of cuts, though I'm still looking for better
alternatives for the whole gpiochipN:offset -> sysfs_nr mapping problem
- see below.
> > > you simply not using
> > > /sys/bus/gpio/devices/$chip ?
> >
> > Cos that shows all the gpiochips, not just the ones created by gpio-mockup.
>
> I didn't get this. What is the content of $chip in your case?
>
$chip is the gpiochipN name, so gpiochip0, gpiochip1 etc.
What we are trying to find here is the base of the GPIO numbering for
the chip so we can export/unexport them to sysfs (after adding the
offset for the particular line).
> > And I certainly don't want to go messing with real hardware.
> > The default tests should still run on real hardware - but only
> > accessing the mockup devices.
> >
> > Got a better way to filter out real hardware?
>
> I probably have to understand what is the input and what is the
> expected output. It's possible I missed something here.
>
> > > > + # e.g. /sys/class/gpio/gpiochip508/device/gpiochip0/dev
> > > > + local syschip=`ls -d $GPIO_SYSFS/gpiochip*/device/$chip/dev`
> > >
> > > ls -d is fragile, better to use `find ...`
> >
> > OK
> >
> > > > + syschip=${syschip#$GPIO_SYSFS}
> > > > + syschip=${syschip%/device/$chip/dev}
> > >
> > > How does this handle more than one gpiochip listed?
> >
> > It is filtered by $chip so there can only be one.
> > Or is that a false assumption?
>
> When you have glob() in use it may return any number of results
> (starting from 0) and your script should be prepared for that.
>
Yeah, we really don't want to be using globs at all.
> > > Also, can you consider optimizing these to get whatever you want easily?
> >
> > Sadly that IS my optimized way - I don't know of an easier way to find
> > the sysfs GPIO number given the gpiochip and offset :-(.
> > Happy to learn of any alternative.
>
> I'm talking about getting $syschip. I think there is a way to get it
> without all those shell substitutions from somewhere else.
>
$syschip is just an intermediate that I'm not really interested in - it
just helps find the base, and so the nr.
I've been playing with alternatives and my current one is:
# e.g. /sys/devices/platform/gpio-mockup.1/gpiochip1
local platform=$(find $SYSFS/devices/platform/ -name $chip -type d
-maxdepth 2)
[ "$platform" ] || fail "can't find platform of $chip"
# e.g. /sys/devices/platform/gpio-mockup.1/gpio/gpiochip508/base
local base=$(find $(dirname $platform)/gpio/ -name base -type f
-maxdepth 2)
[ "$base" ] || fail "can't find base of $chip"
sysfs_nr=$(< $base)
sysfs_nr=$(($sysfs_nr + $offset))
which works, though still doesn't handle the possibility of multiple
matches returned by the finds.
> > > > + sysfs_nr=`cat $SYSFS/devices/$platform/gpio/$syschip/base`
> > >
> > > (It's probably fine here, but this doesn't work against PCI bus, for
> > > example, see above for the fix)
> >
> > Not sure what you mean here.
>
> When GPIO is a PCI device the above won't give a proper path.
> If we wish to give an example to somebody, it would be better to have
> it good enough.
>
How would it appear for PCI bus?
> > > > + sysfs_nr=$(($sysfs_nr + $offset))
> > > > + sysfs_ldir=$GPIO_SYSFS/gpio$sysfs_nr
> > > > }
>
> ...
>
> > > > +set_line()
> > > > {
> > > > + if [ -z "$sysfs_nr&quo
Re: [PATCH 1/7] selftests: gpio: rework and simplify test implementation
On Sun, Jan 03, 2021 at 12:20:26AM +0200, Andy Shevchenko wrote:
> On Sat, Jan 2, 2021 at 4:32 AM Kent Gibson wrote:
> >
> > The GPIO mockup selftests are overly complicated with separate
> > implementations of the tests for sysfs and cdev uAPI, and with the cdev
> > implementation being dependent on tools/gpio and libmount.
> >
> > Rework the test implementation to provide a common test suite with a
> > simplified pluggable uAPI interface. The cdev implementation utilises
> > the GPIO uAPI directly to remove the dependence on tools/gpio.
> > The simplified uAPI interface removes the need for any file system mount
> > checks in C, and so removes the dependence on libmount.
> >
> > The rework also fixes the sysfs test implementation which has been broken
> > since the device created in the multiple gpiochip case was split into
> > separate devices.
>
> Okay, I commented something, not sure if everything is correct, needs
> double checking.
> Shell is quite a hard programming language. Everyday I found something
> new about it.
>
You are telling me - there are about six million ways to do even the
most trivial tasks. Makes you appreciate more constrained languages.
> ...
>
> > +#include
>
> Perhaps include it after system headers?
>
hehe, I blindly sorted them.
Should it matter?
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
>
> ...
>
> > +SYSFS=`mount -t sysfs | head -1 | awk '{ print $3 }'`
>
> Oh, would below be better?
> grep -w sysfs /proc/mounts | cut -f2 -d' '
>
That looks good - the other is a carry over from the old gpio-mockup.sh.
> ...
>
> > +[ ! -d "$SYSFS" ] && skip "sysfs is not mounted"
>
> [ -d ... ] || skip "..."
>
Yeah, those were if [ .. ]; then fi originally. I did the first step
of simplification and missed the second :-(.
> ...
>
> > +[ ! -d "$GPIO_SYSFS" ] && skip "CONFIG_GPIO_SYSFS is not selected"
>
> Ditto.
>
> ...
>
> > + local platform=`cat $SYSFS/kernel/debug/gpio | grep "$chip:" | tr
> > -d ',' | awk '{print $5}'`
>
> Besides useless use of cat (and tr + awk can be simplified) why are
What do you suggest for the tr/awk simplification?
> you simply not using
> /sys/bus/gpio/devices/$chip ?
>
Cos that shows all the gpiochips, not just the ones created by gpio-mockup.
And I certainly don't want to go messing with real hardware.
The default tests should still run on real hardware - but only
accessing the mockup devices.
Got a better way to filter out real hardware?
> > + # e.g. /sys/class/gpio/gpiochip508/device/gpiochip0/dev
> > + local syschip=`ls -d $GPIO_SYSFS/gpiochip*/device/$chip/dev`
>
> ls -d is fragile, better to use `find ...`
>
OK
> > + syschip=${syschip#$GPIO_SYSFS}
> > + syschip=${syschip%/device/$chip/dev}
>
> How does this handle more than one gpiochip listed?
It is filtered by $chip so there can only be one.
Or is that a false assumption?
> Also, can you consider optimizing these to get whatever you want easily?
>
Sadly that IS my optimized way - I don't know of an easier way to find
the sysfs GPIO number given the gpiochip and offset :-(.
Happy to learn of any alternative.
> > + sysfs_nr=`cat $SYSFS/devices/$platform/gpio/$syschip/base`
>
> (It's probably fine here, but this doesn't work against PCI bus, for
> example, see above for the fix)
>
Not sure what you mean here.
> > + sysfs_nr=$(($sysfs_nr + $offset))
> > + sysfs_ldir=$GPIO_SYSFS/gpio$sysfs_nr
> > }
>
> ...
>
> > +set_line()
> > {
> > + if [ -z "$sysfs_nr" ]; then
> > + find_sysfs_nr
> > + echo $sysfs_nr > $GPIO_SYSFS/export
> > fi
>
> It sounds like a separate function (you have release_line(), perhaps
> acquire_line() is good to have).
>
The cdev implementation has to release and re-acquire in the background
as there is no simple way to perform a set_config on a requested line
from shell - just holding the requested line for a set is painful enough,
and the goal here was to keep the tests simple.
I didn't want to make line acquisition/release explicit in the gpio-mockup
tests, as that would make them needlessly complicated, so the acquire is
bundled into the set_line - and anywhere else the uAPI implementation
needs it. There is an implicit assumption that a set_line will always
be called before a get_line, but that is always true - there is no
"as-is" being tested here.
Of course you still ne
Re: [PATCH 1/7] selftests: gpio: rework and simplify test implementation
On Sat, Jan 02, 2021 at 03:52:32PM +0200, Andy Shevchenko wrote: > On Saturday, January 2, 2021, Kent Gibson wrote: > > > The GPIO mockup selftests are overly complicated with separate > > implementations of the tests for sysfs and cdev uAPI, and with the cdev > > implementation being dependent on tools/gpio and libmount. > > > > Rework the test implementation to provide a common test suite with a > > simplified pluggable uAPI interface. The cdev implementation utilises > > the GPIO uAPI directly to remove the dependence on tools/gpio. > > The simplified uAPI interface removes the need for any file system mount > > checks in C, and so removes the dependence on libmount. > > > > The rework also fixes the sysfs test implementation which has been broken > > since the device created in the multiple gpiochip case was split into > > separate devices. > > > > > > I briefly looked at code in shell below... there are places to improve > (useless use of: cat, test, negation, etc). > My shell is clearly pretty poor, so I would really appreciate a pointer to an example of each, and I'll then hunt down the rest. Thanks, Kent.
[PATCH 0/7] selftests: gpio: rework and port to GPIO uAPI v2
Initially I just wanted to port the selftests to the latest GPIO uAPI, but on finding that, due to dependency issues, the selftests are not built for the buildroot environments that I do most of my GPIO testing in, I decided to take a closer look. The first patch is essentially a rewrite of the exising test suite. It uses a simplified abstraction of the uAPI interfaces to allow a common test suite to test the gpio-mockup using either of the uAPI interfaces. The simplified cdev interface is implemented in gpio-mockup.sh, with the actual driving of the uAPI implemented in gpio-mockup-cdev.c. The simplified sysfs interface replaces gpio-mockup-sysfs.sh and is loaded over the cdev implementation when selected. The new tests should also be simpler to extend to cover new mockup interfaces, such as the one Bart has been working on. I have dropped support for testing modules other than gpio-mockup from the command line options, as the tests are very gpio-mockup specific so I didn't see any calling for it. I have also tried to emphasise in the test output that the tests are covering the gpio-mockup itself. They do perform some implicit testing of gpiolib and the uAPI interfaces, and so can be useful as smoke tests for those, but their primary focus is the gpio-mockup. Patches 2 through 5 do some cleaning up that is now possible with the new implementation, including enabling building in buildroot environments. Patch 4 doesn't strictly clean up all the old gpio references that it could - the gpio was the only Level 1 test, so the Level 1 tests could potentially be removed, but I was unsure if there may be other implications to removing a whole test level, or that it may be useful as a placeholder in case other static LDLIBS tests are added in the future?? Patch 6 finally gets around to porting the tests to the latest GPIO uAPI. And Patch 7 updates the config to set the CONFIG_GPIO_CDEV option that was added in v5.10. Cheers, Kent. Kent Gibson (7): selftests: gpio: rework and simplify test implementation selftests: gpio: remove obsolete gpio-mockup-chardev.c selftests: remove obsolete build restriction for gpio selftests: remove obsolete gpio references from kselftest_deps.sh tools: gpio: remove uAPI v1 code no longer used by selftests selftests: gpio: port to GPIO uAPI v2 selftests: gpio: add CONFIG_GPIO_CDEV to config tools/gpio/gpio-utils.c | 89 tools/gpio/gpio-utils.h | 6 - tools/testing/selftests/Makefile | 9 - tools/testing/selftests/gpio/Makefile | 26 +- tools/testing/selftests/gpio/config | 1 + .../testing/selftests/gpio/gpio-mockup-cdev.c | 198 .../selftests/gpio/gpio-mockup-chardev.c | 323 .../selftests/gpio/gpio-mockup-sysfs.sh | 168 ++- tools/testing/selftests/gpio/gpio-mockup.sh | 469 -- tools/testing/selftests/kselftest_deps.sh | 4 +- 10 files changed, 573 insertions(+), 720 deletions(-) create mode 100644 tools/testing/selftests/gpio/gpio-mockup-cdev.c delete mode 100644 tools/testing/selftests/gpio/gpio-mockup-chardev.c -- 2.30.0
[PATCH 7/7] selftests: gpio: add CONFIG_GPIO_CDEV to config
GPIO CDEV is now optional and required for the selftests so add it to the config. Signed-off-by: Kent Gibson --- tools/testing/selftests/gpio/config | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/gpio/config b/tools/testing/selftests/gpio/config index abaa6902b7b6..ce100342c20b 100644 --- a/tools/testing/selftests/gpio/config +++ b/tools/testing/selftests/gpio/config @@ -1,2 +1,3 @@ CONFIG_GPIOLIB=y +CONFIG_GPIO_CDEV=y CONFIG_GPIO_MOCKUP=m -- 2.30.0
[PATCH 2/7] selftests: gpio: remove obsolete gpio-mockup-chardev.c
GPIO selftests have changed to new gpio-mockup-cdev helper, so remove
old gpio-mockup-chardev helper.
Signed-off-by: Kent Gibson
---
.../selftests/gpio/gpio-mockup-chardev.c | 323 --
1 file changed, 323 deletions(-)
delete mode 100644 tools/testing/selftests/gpio/gpio-mockup-chardev.c
diff --git a/tools/testing/selftests/gpio/gpio-mockup-chardev.c
b/tools/testing/selftests/gpio/gpio-mockup-chardev.c
deleted file mode 100644
index 73ead8828d3a..
--- a/tools/testing/selftests/gpio/gpio-mockup-chardev.c
+++ /dev/null
@@ -1,323 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * GPIO chardev test helper
- *
- * Copyright (C) 2016 Bamvor Jian Zhang
- */
-
-#define _GNU_SOURCE
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include "../../../gpio/gpio-utils.h"
-
-#define CONSUMER "gpio-selftest"
-#defineGC_NUM 10
-enum direction {
- OUT,
- IN
-};
-
-static int get_debugfs(char **path)
-{
- struct libmnt_context *cxt;
- struct libmnt_table *tb;
- struct libmnt_iter *itr = NULL;
- struct libmnt_fs *fs;
- int found = 0, ret;
-
- cxt = mnt_new_context();
- if (!cxt)
- err(EXIT_FAILURE, "libmount context allocation failed");
-
- itr = mnt_new_iter(MNT_ITER_FORWARD);
- if (!itr)
- err(EXIT_FAILURE, "failed to initialize libmount iterator");
-
- if (mnt_context_get_mtab(cxt, ))
- err(EXIT_FAILURE, "failed to read mtab");
-
- while (mnt_table_next_fs(tb, itr, ) == 0) {
- const char *type = mnt_fs_get_fstype(fs);
-
- if (!strcmp(type, "debugfs")) {
- found = 1;
- break;
- }
- }
- if (found) {
- ret = asprintf(path, "%s/gpio", mnt_fs_get_target(fs));
- if (ret < 0)
- err(EXIT_FAILURE, "failed to format string");
- }
-
- mnt_free_iter(itr);
- mnt_free_context(cxt);
-
- if (!found)
- return -1;
-
- return 0;
-}
-
-static int gpio_debugfs_get(const char *consumer, int *dir, int *value)
-{
- char *debugfs;
- FILE *f;
- char *line = NULL;
- size_t len = 0;
- char *cur;
- int found = 0;
-
- if (get_debugfs() != 0)
- err(EXIT_FAILURE, "debugfs is not mounted");
-
- f = fopen(debugfs, "r");
- if (!f)
- err(EXIT_FAILURE, "read from gpio debugfs failed");
-
- /*
-* gpio-2 (|gpio-selftest ) in lo
-*/
- while (getline(, , f) != -1) {
- cur = strstr(line, consumer);
- if (cur == NULL)
- continue;
-
- cur = strchr(line, ')');
- if (!cur)
- continue;
-
- cur += 2;
- if (!strncmp(cur, "out", 3)) {
- *dir = OUT;
- cur += 4;
- } else if (!strncmp(cur, "in", 2)) {
- *dir = IN;
- cur += 4;
- }
-
- if (!strncmp(cur, "hi", 2))
- *value = 1;
- else if (!strncmp(cur, "lo", 2))
- *value = 0;
-
- found = 1;
- break;
- }
- free(debugfs);
- fclose(f);
- free(line);
-
- if (!found)
- return -1;
-
- return 0;
-}
-
-static struct gpiochip_info *list_gpiochip(const char *gpiochip_name, int *ret)
-{
- struct gpiochip_info *cinfo;
- struct gpiochip_info *current;
- const struct dirent *ent;
- DIR *dp;
- char *chrdev_name;
- int fd;
- int i = 0;
-
- cinfo = calloc(sizeof(struct gpiochip_info) * 4, GC_NUM + 1);
- if (!cinfo)
- err(EXIT_FAILURE, "gpiochip_info allocation failed");
-
- current = cinfo;
- dp = opendir("/dev");
- if (!dp) {
- *ret = -errno;
- goto error_out;
- } else {
- *ret = 0;
- }
-
- while (ent = readdir(dp), ent) {
- if (check_prefix(ent->d_name, "gpiochip")) {
- *ret = asprintf(_name, "/dev/%s", ent->d_name);
- if (*ret < 0)
- goto error_out;
-
- fd = open(chrdev_name, 0);
- if (fd == -1) {
- *ret = -errno;
- fprintf(stderr, "Failed to open %s\n",
-
[PATCH 6/7] selftests: gpio: port to GPIO uAPI v2
Add a port to the GPIO uAPI v2 interface and make it the default.
Signed-off-by: Kent Gibson
---
.../testing/selftests/gpio/gpio-mockup-cdev.c | 75 +--
tools/testing/selftests/gpio/gpio-mockup.sh | 11 ++-
2 files changed, 76 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/gpio/gpio-mockup-cdev.c
b/tools/testing/selftests/gpio/gpio-mockup-cdev.c
index 3bfd876a8b6a..e8e3d2ec662c 100644
--- a/tools/testing/selftests/gpio/gpio-mockup-cdev.c
+++ b/tools/testing/selftests/gpio/gpio-mockup-cdev.c
@@ -18,6 +18,44 @@
#define CONSUMER "gpio-mockup-cdev"
+static int request_line_v2(int cfd, unsigned int offset,
+ uint64_t flags, unsigned int val)
+{
+ struct gpio_v2_line_request req;
+ int ret;
+
+ memset(, 0, sizeof(req));
+ req.num_lines = 1;
+ req.offsets[0] = offset;
+ req.config.flags = flags;
+ strcpy(req.consumer, CONSUMER);
+ if (flags & GPIO_V2_LINE_FLAG_OUTPUT) {
+ req.config.num_attrs = 1;
+ req.config.attrs[0].mask = 1;
+ req.config.attrs[0].attr.id =
GPIO_V2_LINE_ATTR_ID_OUTPUT_VALUES;
+ if (val)
+ req.config.attrs[0].attr.values = 1;
+ }
+ ret = ioctl(cfd, GPIO_V2_GET_LINE_IOCTL, );
+ if (ret == -1)
+ return -errno;
+ return req.fd;
+}
+
+
+static int get_value_v2(int lfd)
+{
+ struct gpio_v2_line_values vals;
+ int ret;
+
+ memset(, 0, sizeof(vals));
+ vals.mask = 1;
+ ret = ioctl(lfd, GPIO_V2_LINE_GET_VALUES_IOCTL, );
+ if (ret == -1)
+ return -errno;
+ return vals.bits & 0x1;
+}
+
static int request_line_v1(int cfd, unsigned int offset,
uint32_t flags, unsigned int val)
{
@@ -57,6 +95,7 @@ static void usage(char *prog)
printf(" (default is to leave bias unchanged):\n");
printf("-l: set line active low (default is active high)\n");
printf("-s: set line value (default is to get line value)\n");
+ printf("-u: uAPI version to use (default is 2)\n");
exit(-1);
}
@@ -78,29 +117,42 @@ int main(int argc, char *argv[])
{
char *chip;
int opt, ret, cfd, lfd;
- unsigned int offset, val;
+ unsigned int offset, val, abiv;
uint32_t flags_v1;
+ uint64_t flags_v2;
+ abiv = 2;
ret = 0;
flags_v1 = GPIOHANDLE_REQUEST_INPUT;
+ flags_v2 = GPIO_V2_LINE_FLAG_INPUT;
while ((opt = getopt(argc, argv, "lb:s:u:")) != -1) {
switch (opt) {
case 'l':
flags_v1 |= GPIOHANDLE_REQUEST_ACTIVE_LOW;
+ flags_v2 |= GPIO_V2_LINE_FLAG_ACTIVE_LOW;
break;
case 'b':
- if (strcmp("pull-up", optarg) == 0)
+ if (strcmp("pull-up", optarg) == 0) {
flags_v1 |= GPIOHANDLE_REQUEST_BIAS_PULL_UP;
- else if (strcmp("pull-down", optarg) == 0)
+ flags_v2 |= GPIO_V2_LINE_FLAG_BIAS_PULL_UP;
+ } else if (strcmp("pull-down", optarg) == 0) {
flags_v1 |= GPIOHANDLE_REQUEST_BIAS_PULL_DOWN;
- else if (strcmp("disabled", optarg) == 0)
+ flags_v2 |= GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN;
+ } else if (strcmp("disabled", optarg) == 0) {
flags_v1 |= GPIOHANDLE_REQUEST_BIAS_DISABLE;
+ flags_v2 |= GPIO_V2_LINE_FLAG_BIAS_DISABLED;
+ }
break;
case 's':
val = atoi(optarg);
flags_v1 &= ~GPIOHANDLE_REQUEST_INPUT;
flags_v1 |= GPIOHANDLE_REQUEST_OUTPUT;
+ flags_v2 &= ~GPIO_V2_LINE_FLAG_INPUT;
+ flags_v2 |= GPIO_V2_LINE_FLAG_OUTPUT;
+ break;
+ case 'u':
+ abiv = atoi(optarg);
break;
default:
usage(argv[0]);
@@ -119,7 +171,10 @@ int main(int argc, char *argv[])
return -errno;
}
- lfd = request_line_v1(cfd, offset, flags_v1, val);
+ if (abiv == 1)
+ lfd = request_line_v1(cfd, offset, flags_v1, val);
+ else
+ lfd = request_line_v2(cfd, offset, flags_v2, val);
close(cfd);
@@ -128,10 +183,14 @@ int main(int argc, char *argv[])
return lfd;
}
- if (flags_v1 & GPIOHANDLE_REQUEST_OUTPUT)
+ if (flags_v2 & GPIO_V2_LINE_FL
[PATCH 4/7] selftests: remove obsolete gpio references from kselftest_deps.sh
GPIO Makefile has been greatly simplified so remove references to lines
which no longer exist.
Signed-off-by: Kent Gibson
---
tools/testing/selftests/kselftest_deps.sh | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kselftest_deps.sh
b/tools/testing/selftests/kselftest_deps.sh
index bbc04646346b..00e60d6eb16b 100755
--- a/tools/testing/selftests/kselftest_deps.sh
+++ b/tools/testing/selftests/kselftest_deps.sh
@@ -129,13 +129,11 @@ l2_tests=$(grep -r --include=Makefile ": LDLIBS" | \
grep -v "VAR_LDLIBS" | awk -F: '{print $1}')
# Level 3
-# gpio, memfd and others use pkg-config to find mount and fuse libs
+# memfd and others use pkg-config to find mount and fuse libs
# respectively and save it in VAR_LDLIBS. If pkg-config doesn't find
# any, VAR_LDLIBS set to default.
# Use the default value and filter out pkg-config for dependency check.
# e.g:
-# gpio/Makefile
-# VAR_LDLIBS := $(shell pkg-config --libs mount) 2>/dev/null)
# memfd/Makefile
# VAR_LDLIBS := $(shell pkg-config fuse --libs 2>/dev/null)
--
2.30.0
[PATCH 3/7] selftests: remove obsolete build restriction for gpio
Build restrictions related to the gpio-mockup-chardev helper are no longer relevant so remove them. Signed-off-by: Kent Gibson --- tools/testing/selftests/Makefile | 9 - 1 file changed, 9 deletions(-) diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index d9c283503159..5411041e63a0 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -121,15 +121,6 @@ ARCH ?= $(SUBARCH) export KSFT_KHDR_INSTALL_DONE := 1 export BUILD -# build and run gpio when output directory is the src dir. -# gpio has dependency on tools/gpio and builds tools/gpio -# objects in the src directory in all cases making the src -# repo dirty even when objects are relocated. -ifneq (1,$(DEFAULT_INSTALL_HDR_PATH)) - TMP := $(filter-out gpio, $(TARGETS)) - TARGETS := $(TMP) -endif - # set default goal to all, so make without a target runs all, even when # all isn't the first target in the file. .DEFAULT_GOAL := all -- 2.30.0
[PATCH 5/7] tools: gpio: remove uAPI v1 code no longer used by selftests
gpio-mockup-chardev helper has been obsoleted and removed, so also remove
the tools/gpio code that it, and nothing else, was using.
Signed-off-by: Kent Gibson
---
tools/gpio/gpio-utils.c | 89 -
tools/gpio/gpio-utils.h | 6 ---
2 files changed, 95 deletions(-)
diff --git a/tools/gpio/gpio-utils.c b/tools/gpio/gpio-utils.c
index 37187e056c8b..1639b4d832cd 100644
--- a/tools/gpio/gpio-utils.c
+++ b/tools/gpio/gpio-utils.c
@@ -32,74 +32,6 @@
* following api will request gpio lines, do the operation and then
* release these lines.
*/
-/**
- * gpiotools_request_linehandle() - request gpio lines in a gpiochip
- * @device_name: The name of gpiochip without prefix "/dev/",
- * such as "gpiochip0"
- * @lines: An array desired lines, specified by offset
- * index for the associated GPIO device.
- * @num_lines: The number of lines to request.
- * @flag: The new flag for requsted gpio. Reference
- * "linux/gpio.h" for the meaning of flag.
- * @data: Default value will be set to gpio when flag is
- * GPIOHANDLE_REQUEST_OUTPUT.
- * @consumer_label:The name of consumer, such as "sysfs",
- * "powerkey". This is useful for other users to
- * know who is using.
- *
- * Request gpio lines through the ioctl provided by chardev. User
- * could call gpiotools_set_values() and gpiotools_get_values() to
- * read and write respectively through the returned fd. Call
- * gpiotools_release_linehandle() to release these lines after that.
- *
- * Return: On success return the fd;
- * On failure return the errno.
- */
-int gpiotools_request_linehandle(const char *device_name, unsigned int *lines,
-unsigned int num_lines, unsigned int flag,
-struct gpiohandle_data *data,
-const char *consumer_label)
-{
- struct gpiohandle_request req;
- char *chrdev_name;
- int fd;
- int i;
- int ret;
-
- ret = asprintf(_name, "/dev/%s", device_name);
- if (ret < 0)
- return -ENOMEM;
-
- fd = open(chrdev_name, 0);
- if (fd == -1) {
- ret = -errno;
- fprintf(stderr, "Failed to open %s, %s\n",
- chrdev_name, strerror(errno));
- goto exit_free_name;
- }
-
- for (i = 0; i < num_lines; i++)
- req.lineoffsets[i] = lines[i];
-
- req.flags = flag;
- strcpy(req.consumer_label, consumer_label);
- req.lines = num_lines;
- if (flag & GPIOHANDLE_REQUEST_OUTPUT)
- memcpy(req.default_values, data, sizeof(req.default_values));
-
- ret = ioctl(fd, GPIO_GET_LINEHANDLE_IOCTL, );
- if (ret == -1) {
- ret = -errno;
- fprintf(stderr, "Failed to issue %s (%d), %s\n",
- "GPIO_GET_LINEHANDLE_IOCTL", ret, strerror(errno));
- }
-
- if (close(fd) == -1)
- perror("Failed to close GPIO character device file");
-exit_free_name:
- free(chrdev_name);
- return ret < 0 ? ret : req.fd;
-}
/**
* gpiotools_request_line() - request gpio lines in a gpiochip
@@ -215,27 +147,6 @@ int gpiotools_get_values(const int fd, struct
gpio_v2_line_values *values)
return ret;
}
-/**
- * gpiotools_release_linehandle(): Release the line(s) of gpiochip
- * @fd:The fd returned by
- * gpiotools_request_linehandle().
- *
- * Return: On success return 0;
- * On failure return the errno.
- */
-int gpiotools_release_linehandle(const int fd)
-{
- int ret;
-
- ret = close(fd);
- if (ret == -1) {
- perror("Failed to close GPIO LINEHANDLE device file");
- ret = -errno;
- }
-
- return ret;
-}
-
/**
* gpiotools_release_line(): Release the line(s) of gpiochip
* @fd:The fd returned by
diff --git a/tools/gpio/gpio-utils.h b/tools/gpio/gpio-utils.h
index 6c69a9f1c253..8af7c8ee19ce 100644
--- a/tools/gpio/gpio-utils.h
+++ b/tools/gpio/gpio-utils.h
@@ -24,12 +24,6 @@ static inline int check_prefix(const char *str, const char
*prefix)
strncmp(str, prefix, strlen(prefix)) == 0;
}
-int gpiotools_request_linehandle(const char *device_name, unsigned int *lines,
-unsigned int num_lines, unsigned int flag,
-struct gpiohandle_data *data,
-const char *consumer_label);
-int gpiotools_release_linehandle(const int fd);
-
int gpiotools_request_line(const char *device_name,
unsigned int *lines,
unsigned int num_lines,
--
2.30.0
[PATCH 1/7] selftests: gpio: rework and simplify test implementation
The GPIO mockup selftests are overly complicated with separate
implementations of the tests for sysfs and cdev uAPI, and with the cdev
implementation being dependent on tools/gpio and libmount.
Rework the test implementation to provide a common test suite with a
simplified pluggable uAPI interface. The cdev implementation utilises
the GPIO uAPI directly to remove the dependence on tools/gpio.
The simplified uAPI interface removes the need for any file system mount
checks in C, and so removes the dependence on libmount.
The rework also fixes the sysfs test implementation which has been broken
since the device created in the multiple gpiochip case was split into
separate devices.
Fixes: commit 8a39f597bcfd ("gpio: mockup: rework device probing")
Signed-off-by: Kent Gibson
---
tools/testing/selftests/gpio/Makefile | 26 +-
.../testing/selftests/gpio/gpio-mockup-cdev.c | 139 ++
.../selftests/gpio/gpio-mockup-sysfs.sh | 168 ++-
tools/testing/selftests/gpio/gpio-mockup.sh | 462 --
4 files changed, 505 insertions(+), 290 deletions(-)
create mode 100644 tools/testing/selftests/gpio/gpio-mockup-cdev.c
diff --git a/tools/testing/selftests/gpio/Makefile
b/tools/testing/selftests/gpio/Makefile
index 32bdc978a711..e4363c64d40d 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,32 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
-VAR_CFLAGS := $(shell pkg-config --cflags mount 2>/dev/null)
-VAR_LDLIBS := $(shell pkg-config --libs mount 2>/dev/null)
-ifeq ($(VAR_LDLIBS),)
-VAR_LDLIBS := -lmount -I/usr/include/libmount
-endif
-
-CFLAGS += -O2 -g -std=gnu99 -Wall -I../../../../usr/include/ $(VAR_CFLAGS)
-LDLIBS += $(VAR_LDLIBS)
-
TEST_PROGS := gpio-mockup.sh
TEST_FILES := gpio-mockup-sysfs.sh
-TEST_PROGS_EXTENDED := gpio-mockup-chardev
-
-GPIODIR := $(realpath ../../../gpio)
-GPIOOBJ := gpio-utils.o
+TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev
-all: $(TEST_PROGS_EXTENDED)
-
-override define CLEAN
- $(RM) $(TEST_PROGS_EXTENDED)
- $(MAKE) -C $(GPIODIR) OUTPUT=$(GPIODIR)/ clean
-endef
-
-KSFT_KHDR_INSTALL := 1
include ../lib.mk
-$(TEST_PROGS_EXTENDED): $(GPIODIR)/$(GPIOOBJ)
-
-$(GPIODIR)/$(GPIOOBJ):
- $(MAKE) OUTPUT=$(GPIODIR)/ -C $(GPIODIR)
diff --git a/tools/testing/selftests/gpio/gpio-mockup-cdev.c
b/tools/testing/selftests/gpio/gpio-mockup-cdev.c
new file mode 100644
index ..3bfd876a8b6a
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-mockup-cdev.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * GPIO mockup cdev test helper
+ *
+ * Copyright (C) 2020 Kent Gibson
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define CONSUMER "gpio-mockup-cdev"
+
+static int request_line_v1(int cfd, unsigned int offset,
+ uint32_t flags, unsigned int val)
+{
+ struct gpiohandle_request req;
+ int ret;
+
+ memset(, 0, sizeof(req));
+ req.lines = 1;
+ req.lineoffsets[0] = offset;
+ req.flags = flags;
+ strcpy(req.consumer_label, CONSUMER);
+ if (flags & GPIOHANDLE_REQUEST_OUTPUT)
+ req.default_values[0] = val;
+
+ ret = ioctl(cfd, GPIO_GET_LINEHANDLE_IOCTL, );
+ if (ret == -1)
+ return -errno;
+ return req.fd;
+}
+
+static int get_value_v1(int lfd)
+{
+ struct gpiohandle_data vals;
+ int ret;
+
+ memset(, 0, sizeof(vals));
+ ret = ioctl(lfd, GPIOHANDLE_GET_LINE_VALUES_IOCTL, );
+ if (ret == -1)
+ return -errno;
+ return vals.values[0];
+}
+
+static void usage(char *prog)
+{
+ printf("Usage: %s [-l] [-b ] [-s ] [-u ]
\n", prog);
+ printf("-b: set line bias to one of pull-down, pull-up,
disabled\n");
+ printf(" (default is to leave bias unchanged):\n");
+ printf("-l: set line active low (default is active high)\n");
+ printf("-s: set line value (default is to get line value)\n");
+ exit(-1);
+}
+
+static int wait_signal(void)
+{
+ int sig;
+ sigset_t wset;
+
+ sigemptyset();
+ sigaddset(, SIGHUP);
+ sigaddset(, SIGINT);
+ sigaddset(, SIGTERM);
+ sigwait(, );
+
+ return sig;
+}
+
+int main(int argc, char *argv[])
+{
+ char *chip;
+ int opt, ret, cfd, lfd;
+ unsigned int offset, val;
+ uint32_t flags_v1;
+
+ ret = 0;
+ flags_v1 = GPIOHANDLE_REQUEST_INPUT;
+
+ while ((opt = getopt(argc, argv, "lb:s:u:")) != -1) {
+ switch (opt) {
+ case 'l':
+ flags_v1 |= GPIOHANDLE_REQUEST_ACTIVE_LOW;
+ break;
+ case 'b':
+ if (strcmp("pull-up", optarg) == 0)
+
[PATCH] gpiolib: cdev: fix frame size warning in gpio_ioctl()
The kernel test robot reports the following warning in [1]:
drivers/gpio/gpiolib-cdev.c: In function 'gpio_ioctl':
>>drivers/gpio/gpiolib-cdev.c:1437:1: warning: the frame size of 1040 bytes is
>>larger than 1024 bytes [-Wframe-larger-than=]
Refactor gpio_ioctl() to handle each ioctl in its own helper function
and so reduce the variables stored on the stack to those explicitly
required to service the ioctl at hand.
The lineinfo_get_v1() helper handles both the GPIO_GET_LINEINFO_IOCTL
and GPIO_GET_LINEINFO_WATCH_IOCTL, as per the corresponding v2
implementation - lineinfo_get().
[1] https://lore.kernel.org/lkml/202012270910.vw3qc1er-...@intel.com/
Fixes: aad955842d1c ("gpiolib: cdev: support GPIO_V2_GET_LINEINFO_IOCTL and
GPIO_V2_GET_LINEINFO_WATCH_IOCTL")
Reported-by: kernel test robot
Signed-off-by: Kent Gibson
---
drivers/gpio/gpiolib-cdev.c | 145 ++--
1 file changed, 73 insertions(+), 72 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 12b679ca552c..1a7b51163528 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -1979,6 +1979,21 @@ struct gpio_chardev_data {
#endif
};
+static int chipinfo_get(struct gpio_chardev_data *cdev, void __user *ip)
+{
+ struct gpio_device *gdev = cdev->gdev;
+ struct gpiochip_info chipinfo;
+
+ memset(, 0, sizeof(chipinfo));
+
+ strscpy(chipinfo.name, dev_name(>dev), sizeof(chipinfo.name));
+ strscpy(chipinfo.label, gdev->label, sizeof(chipinfo.label));
+ chipinfo.lines = gdev->ngpio;
+ if (copy_to_user(ip, , sizeof(chipinfo)))
+ return -EFAULT;
+ return 0;
+}
+
#ifdef CONFIG_GPIO_CDEV_V1
/*
* returns 0 if the versions match, else the previously selected ABI version
@@ -1993,6 +2008,41 @@ static int lineinfo_ensure_abi_version(struct
gpio_chardev_data *cdata,
return abiv;
}
+
+static int lineinfo_get_v1(struct gpio_chardev_data *cdev, void __user *ip,
+ bool watch)
+{
+ struct gpio_desc *desc;
+ struct gpioline_info lineinfo;
+ struct gpio_v2_line_info lineinfo_v2;
+
+ if (copy_from_user(, ip, sizeof(lineinfo)))
+ return -EFAULT;
+
+ /* this doubles as a range check on line_offset */
+ desc = gpiochip_get_desc(cdev->gdev->chip, lineinfo.line_offset);
+ if (IS_ERR(desc))
+ return PTR_ERR(desc);
+
+ if (watch) {
+ if (lineinfo_ensure_abi_version(cdev, 1))
+ return -EPERM;
+
+ if (test_and_set_bit(lineinfo.line_offset, cdev->watched_lines))
+ return -EBUSY;
+ }
+
+ gpio_desc_to_lineinfo(desc, _v2);
+ gpio_v2_line_info_to_v1(_v2, );
+
+ if (copy_to_user(ip, , sizeof(lineinfo))) {
+ if (watch)
+ clear_bit(lineinfo.line_offset, cdev->watched_lines);
+ return -EFAULT;
+ }
+
+ return 0;
+}
#endif
static int lineinfo_get(struct gpio_chardev_data *cdev, void __user *ip,
@@ -2030,6 +2080,22 @@ static int lineinfo_get(struct gpio_chardev_data *cdev,
void __user *ip,
return 0;
}
+static int lineinfo_unwatch(struct gpio_chardev_data *cdev, void __user *ip)
+{
+ __u32 offset;
+
+ if (copy_from_user(, ip, sizeof(offset)))
+ return -EFAULT;
+
+ if (offset >= cdev->gdev->ngpio)
+ return -EINVAL;
+
+ if (!test_and_clear_bit(offset, cdev->watched_lines))
+ return -EBUSY;
+
+ return 0;
+}
+
/*
* gpio_ioctl() - ioctl handler for the GPIO chardev
*/
@@ -2037,80 +2103,24 @@ static long gpio_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
{
struct gpio_chardev_data *cdev = file->private_data;
struct gpio_device *gdev = cdev->gdev;
- struct gpio_chip *gc = gdev->chip;
void __user *ip = (void __user *)arg;
- __u32 offset;
/* We fail any subsequent ioctl():s when the chip is gone */
- if (!gc)
+ if (!gdev->chip)
return -ENODEV;
/* Fill in the struct and pass to userspace */
if (cmd == GPIO_GET_CHIPINFO_IOCTL) {
- struct gpiochip_info chipinfo;
-
- memset(, 0, sizeof(chipinfo));
-
- strscpy(chipinfo.name, dev_name(>dev),
- sizeof(chipinfo.name));
- strscpy(chipinfo.label, gdev->label,
- sizeof(chipinfo.label));
- chipinfo.lines = gdev->ngpio;
- if (copy_to_user(ip, , sizeof(chipinfo)))
- return -EFAULT;
- return 0;
+ return chipinfo_get(cdev, ip);
#ifdef CONFIG_GPIO_CDEV_V1
- } else if (cmd == GPIO_GET_LINEINFO_IOCTL) {
- struct gpio_desc *desc;
- stru
[PATCH] gpiolib: fix sysfs when cdev is not selected
In gpiochip_setup_dev() the call to gpiolib_cdev_register() indirectly
calls device_add(). This is still required for the sysfs even when
CONFIG_GPIO_CDEV is not selected in the build.
Replace the stubbed functions in gpiolib-cdev.h with macros in gpiolib.c
that perform the required device_add() and device_del() when
CONFIG_GPIO_CDEV is not selected.
Fixes: d143493c01b7 (gpiolib: make cdev a build option)
Reported-by: Nicolas Schichan
Signed-off-by: Kent Gibson
---
drivers/gpio/gpiolib-cdev.h | 15 ---
drivers/gpio/gpiolib.c | 18 +++---
2 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.h b/drivers/gpio/gpiolib-cdev.h
index cb41dd757338..b42644cbffb8 100644
--- a/drivers/gpio/gpiolib-cdev.h
+++ b/drivers/gpio/gpiolib-cdev.h
@@ -7,22 +7,7 @@
struct gpio_device;
-#ifdef CONFIG_GPIO_CDEV
-
int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt);
void gpiolib_cdev_unregister(struct gpio_device *gdev);
-#else
-
-static inline int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
-{
- return 0;
-}
-
-static inline void gpiolib_cdev_unregister(struct gpio_device *gdev)
-{
-}
-
-#endif /* CONFIG_GPIO_CDEV */
-
#endif /* GPIOLIB_CDEV_H */
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 8e29a60c3697..c980ddcda833 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -480,11 +480,23 @@ static void gpiodevice_release(struct device *dev)
kfree(gdev);
}
+#ifdef CONFIG_GPIO_CDEV
+#define gcdev_register(gdev, devt) gpiolib_cdev_register((gdev), (devt))
+#define gcdev_unregister(gdev) gpiolib_cdev_unregister((gdev))
+#else
+/*
+ * gpiolib_cdev_register() indirectly calls device_add(), which is still
+ * required even when cdev is not selected.
+ */
+#define gcdev_register(gdev, devt) device_add(&(gdev)->dev)
+#define gcdev_unregister(gdev) device_del(&(gdev)->dev)
+#endif
+
static int gpiochip_setup_dev(struct gpio_device *gdev)
{
int ret;
- ret = gpiolib_cdev_register(gdev, gpio_devt);
+ ret = gcdev_register(gdev, gpio_devt);
if (ret)
return ret;
@@ -500,7 +512,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
return 0;
err_remove_device:
- gpiolib_cdev_unregister(gdev);
+ gcdev_unregister(gdev);
return ret;
}
@@ -825,7 +837,7 @@ void gpiochip_remove(struct gpio_chip *gc)
* be removed, else it will be dangling until the last user is
* gone.
*/
- gpiolib_cdev_unregister(gdev);
+ gcdev_unregister(gdev);
put_device(>dev);
}
EXPORT_SYMBOL_GPL(gpiochip_remove);
--
2.29.2
Re: [PATCH v2 0/3] gpiolib: cdev: allow edge event timestamps to be configured as REALTIME
On Fri, Oct 30, 2020 at 03:52:24PM +0100, Bartosz Golaszewski wrote: > On Fri, Oct 30, 2020 at 3:49 PM Bartosz Golaszewski > wrote: > > > > On Thu, Oct 29, 2020 at 12:22 AM Kent Gibson wrote: > > > > > > On Wed, Oct 28, 2020 at 05:01:49PM +0100, Linus Walleij wrote: > > > > On Thu, Oct 15, 2020 at 1:12 AM Kent Gibson > > > > wrote: > > > > > > > > > This patch set adds the option to select CLOCK_REALTIME as the source > > > > > clock for line events. > > > > > > > > > > The first patch is the core of the change, while the remaining two > > > > > update > > > > > the GPIO tools to make use of the new option. > > > > > > > > > > Changes for v2: > > > > > - change line_event_timestamp() return to u64 to avoid clipping to > > > > > 32bits > > > > >on 32bit platforms. > > > > > - fix the line spacing after line_event_timestamp() > > > > > > > > Where are we standing with this patch set? Good to go so > > > > I should just try to merge it? > > > > > > > > > > I'm fine with it, especially now that I've tested it on 32bit platforms > > > as well as 64bit. > > > > > > Bart was ok with v1, and I doubt the changes for v2 would negatively > > > impact that, though I did overlook adding his review tag. > > > > > > Cheers, > > > Kent. > > > > > > > Yours, > > > > Linus Walleij > > > > I'll take it through my tree then. > > > > Bartosz > > The series no longer applies on top of v5.10-rc1. Could you rebase and resend? > Nuts, it relies on my doc tidy-up series that Linus has pulled into fixes, and so will likely go into v5.10-rc2?? Specifically it is based over/conflicts with: 2cc522d3931b gpio: uapi: kernel-doc formatting improvements If I rebase it onto devel then you will get a conflict when those merge. Is that what you want? Cheers, Kent.
[PATCH] gpiolib: cdev: add GPIO_V2_LINE_FLAG_EDGE_BOTH and use it in edge_irq_thread()
Add GPIO_V2_LINE_FLAG_EDGE_BOTH macro and use it in edge_irq_thread() to
improve readability of edge handling cases.
Suggested-by: Andy Shevchenko
Signed-off-by: Kent Gibson
---
drivers/gpio/gpiolib-cdev.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index ea787eb3810d..5eb4435afa64 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -510,6 +510,8 @@ struct linereq {
(GPIO_V2_LINE_FLAG_EDGE_RISING | \
GPIO_V2_LINE_FLAG_EDGE_FALLING)
+#define GPIO_V2_LINE_FLAG_EDGE_BOTH GPIO_V2_LINE_EDGE_FLAGS
+
#define GPIO_V2_LINE_VALID_FLAGS \
(GPIO_V2_LINE_FLAG_ACTIVE_LOW | \
GPIO_V2_LINE_DIRECTION_FLAGS | \
@@ -569,8 +571,7 @@ static irqreturn_t edge_irq_thread(int irq, void *p)
line->timestamp_ns = 0;
eflags = READ_ONCE(line->eflags);
- if (eflags == (GPIO_V2_LINE_FLAG_EDGE_RISING |
- GPIO_V2_LINE_FLAG_EDGE_FALLING)) {
+ if (eflags == GPIO_V2_LINE_FLAG_EDGE_BOTH) {
int level = gpiod_get_value_cansleep(line->desc);
if (level)
--
2.29.0
Re: [PATCH v2 0/3] gpiolib: cdev: allow edge event timestamps to be configured as REALTIME
On Wed, Oct 28, 2020 at 05:01:49PM +0100, Linus Walleij wrote: > On Thu, Oct 15, 2020 at 1:12 AM Kent Gibson wrote: > > > This patch set adds the option to select CLOCK_REALTIME as the source > > clock for line events. > > > > The first patch is the core of the change, while the remaining two update > > the GPIO tools to make use of the new option. > > > > Changes for v2: > > - change line_event_timestamp() return to u64 to avoid clipping to 32bits > >on 32bit platforms. > > - fix the line spacing after line_event_timestamp() > > Where are we standing with this patch set? Good to go so > I should just try to merge it? > I'm fine with it, especially now that I've tested it on 32bit platforms as well as 64bit. Bart was ok with v1, and I doubt the changes for v2 would negatively impact that, though I did overlook adding his review tag. Cheers, Kent. > Yours, > Linus Walleij
Re: [PATCH v2] gpiolib: cdev: document that line eflags are shared
On Fri, Oct 16, 2020 at 05:24:14PM +0300, Andy Shevchenko wrote:
> On Wed, Oct 14, 2020 at 12:21 PM Kent Gibson wrote:
> >
> > The line.eflags field is shared so document this fact and highlight it
> > throughout using READ_ONCE() and WRITE_ONCE() accessors.
> >
> > Also use a local copy of the eflags in edge_irq_thread() to ensure
> > consistent control flow even if eflags changes. This is only a defensive
> > measure as edge_irq_thread() is currently disabled when the eflags are
> > changed.
>
> > - if (line->eflags == (GPIO_V2_LINE_FLAG_EDGE_RISING |
> > -GPIO_V2_LINE_FLAG_EDGE_FALLING)) {
> > + eflags = READ_ONCE(line->eflags);
> > + if (eflags == (GPIO_V2_LINE_FLAG_EDGE_RISING |
> > + GPIO_V2_LINE_FLAG_EDGE_FALLING)) {
>
> Hmm... side note: perhaps at some point
>
> #define GPIO_V2_LINE_FLAG_EDGE_BOTH \
> (GPIO_V2_LINE_FLAG_EDGE_RISING | GPIO_V2_LINE_FLAG_EDGE_FALLING)
>
>if (eflags == GPIO_V2_LINE_FLAG_EDGE_BOTH) {
>
> ?
Yeah, that would make sense. I think I used GPIO_V2_LINE_EDGE_FLAGS,
which is defined the same as your GPIO_V2_LINE_FLAG_EDGE_BOTH, here at
some point, but that just looked wrong.
The GPIO_V2_LINE_FLAG_EDGE_BOTH does read better. I'll add it to the
todo list.
Cheers,
Kent.
Re: [PATCH v2 1/3] gpiolib: cdev: allow edge event timestamps to be configured as REALTIME
On Fri, Oct 16, 2020 at 05:13:22PM +0300, Andy Shevchenko wrote:
> On Thu, Oct 15, 2020 at 6:53 AM Kent Gibson wrote:
> >
> > Using CLOCK_REALTIME as the source for event timestamps is crucial for
> > some specific applications, particularly those requiring timetamps
> > relative to a PTP clock, so provide an option to switch the event
> > timestamp source from the default CLOCK_MONOTONIC to CLOCK_REALTIME.
> >
> > Note that CLOCK_REALTIME was the default source clock for GPIO until
> > Linux 5.7 when it was changed to CLOCK_MONOTONIC due to issues with the
> > shifting of the realtime clock.
> > Providing this option maintains the CLOCK_MONOTONIC as the default,
> > while also providing a path forward for those dependent on the pre-5.7
> > behaviour.
>
> ...
>
> > GPIO_V2_LINE_DIRECTION_FLAGS | \
> > GPIO_V2_LINE_DRIVE_FLAGS | \
> > GPIO_V2_LINE_EDGE_FLAGS | \
> > +GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME | \
>
> Wondering if we would have something like
>
> GPIO_V2_LINE_CLOCK_FLAGS | \
>
> here for the sake of consistency.
>
I considered it, but thought the chance of there ever being another
CLOCK flag to be near zero - the remaining clocks relate to CPU or
thread time which don't have any relevance for external events like
GPIO. If there ever is one we can split it out then.
And it is consistent with the GPIO_V2_LINE_FLAG_ACTIVE_LOW flag that you
pruned out in your response. i.e. lone flags don't get grouped.
> > GPIO_V2_LINE_BIAS_FLAGS)
>
> ...
>
> > +static u64 line_event_timestamp(struct line *line)
> > +{
>
> > + if (test_bit(FLAG_EVENT_CLOCK_REALTIME, >desc->flags))
>
> I dunno if we can actually drop the word EVENT from these definitions.
> I don't think we would have in the near future something similar for
> the non-event data.
>
I considered this too, as another clock flag seems unlikely.
But if we ever add a non-event clock flag then this one would become
confusing, and the overhead of the long name seemed minor compared to
the clarity it brings with it.
Cheers,
Kent.
Re: [PATCH 0/5] gpio: uapi: documentation improvements
On Wed, Oct 14, 2020 at 08:14:20PM +0300, Andy Shevchenko wrote: > On Tue, Oct 13, 2020 at 7:34 PM Kent Gibson wrote: > > On Tue, Oct 13, 2020 at 03:21:47PM +0200, Linus Walleij wrote: > > > On Thu, Oct 8, 2020 at 5:46 PM Bartosz Golaszewski > > > wrote: > > ... > > > > I am waiting for Kent to respin them addressing Andy's comments > > > on patch 5/5 then they can go in as fixes I think. > > > > > > > I had replied to Andy's comments - I'm prefer with my version than his > > suggestion: > > > > "I'm not keen on that alternative as what it suggests is actually a > > pointer comparison, and even if the user realizes that they may instead > > use "strlen(label) == 0", when they shouldn't be assuming that a null > > terminator is present in the array. I avoided mentioning "string" and > > kept it in terms of the char array for the same reason." > > My point is to make documentation less cryptic (= less programming > language stylish). > If you can rephrase it that way - nice! Otherwise, I leave this to Linus. > I agree with your point - including code snippets should be a last resort. But sometimes that is the most effective way to do it. Cheers, Kent.
[PATCH v2 2/3] tools: gpio: add support for reporting realtime event clock to lsgpio
Add support for reporting if a line is configured to report realtime
timestamps in events.
Signed-off-by: Kent Gibson
---
tools/gpio/lsgpio.c | 4
1 file changed, 4 insertions(+)
diff --git a/tools/gpio/lsgpio.c b/tools/gpio/lsgpio.c
index 5a05a454d0c9..c61d061247e1 100644
--- a/tools/gpio/lsgpio.c
+++ b/tools/gpio/lsgpio.c
@@ -65,6 +65,10 @@ struct gpio_flag flagnames[] = {
.name = "bias-disabled",
.mask = GPIO_V2_LINE_FLAG_BIAS_DISABLED,
},
+ {
+ .name = "clock-realtime",
+ .mask = GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME,
+ },
};
static void print_attributes(struct gpio_v2_line_info *info)
--
2.28.0
[PATCH v2 0/3] gpiolib: cdev: allow edge event timestamps to be configured as REALTIME
This patch set adds the option to select CLOCK_REALTIME as the source clock for line events. The first patch is the core of the change, while the remaining two update the GPIO tools to make use of the new option. Changes for v2: - change line_event_timestamp() return to u64 to avoid clipping to 32bits on 32bit platforms. - fix the line spacing after line_event_timestamp() Kent Gibson (3): gpiolib: cdev: allow edge event timestamps to be configured as REALTIME tools: gpio: add support for reporting realtime event clock to lsgpio tools: gpio: add option to report wall-clock time to gpio-event-mon drivers/gpio/gpiolib-cdev.c | 21 ++--- drivers/gpio/gpiolib.h | 1 + include/uapi/linux/gpio.h | 12 +--- tools/gpio/gpio-event-mon.c | 6 +- tools/gpio/lsgpio.c | 4 5 files changed, 37 insertions(+), 7 deletions(-) -- 2.28.0
[PATCH v2 1/3] gpiolib: cdev: allow edge event timestamps to be configured as REALTIME
Using CLOCK_REALTIME as the source for event timestamps is crucial for
some specific applications, particularly those requiring timetamps
relative to a PTP clock, so provide an option to switch the event
timestamp source from the default CLOCK_MONOTONIC to CLOCK_REALTIME.
Note that CLOCK_REALTIME was the default source clock for GPIO until
Linux 5.7 when it was changed to CLOCK_MONOTONIC due to issues with the
shifting of the realtime clock.
Providing this option maintains the CLOCK_MONOTONIC as the default,
while also providing a path forward for those dependent on the pre-5.7
behaviour.
Suggested-by: Jack Winch
Signed-off-by: Kent Gibson
---
drivers/gpio/gpiolib-cdev.c | 21 ++---
drivers/gpio/gpiolib.h | 1 +
include/uapi/linux/gpio.h | 12 +---
3 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 678de9264617..ea787eb3810d 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -515,6 +515,7 @@ struct linereq {
GPIO_V2_LINE_DIRECTION_FLAGS | \
GPIO_V2_LINE_DRIVE_FLAGS | \
GPIO_V2_LINE_EDGE_FLAGS | \
+GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME | \
GPIO_V2_LINE_BIAS_FLAGS)
static void linereq_put_event(struct linereq *lr,
@@ -535,6 +536,14 @@ static void linereq_put_event(struct linereq *lr,
pr_debug_ratelimited("event FIFO is full - event dropped\n");
}
+static u64 line_event_timestamp(struct line *line)
+{
+ if (test_bit(FLAG_EVENT_CLOCK_REALTIME, >desc->flags))
+ return ktime_get_real_ns();
+
+ return ktime_get_ns();
+}
+
static irqreturn_t edge_irq_thread(int irq, void *p)
{
struct line *line = p;
@@ -553,7 +562,7 @@ static irqreturn_t edge_irq_thread(int irq, void *p)
* which case we didn't get the timestamp from
* edge_irq_handler().
*/
- le.timestamp_ns = ktime_get_ns();
+ le.timestamp_ns = line_event_timestamp(line);
if (lr->num_lines != 1)
line->req_seqno = atomic_inc_return(>seqno);
}
@@ -598,7 +607,7 @@ static irqreturn_t edge_irq_handler(int irq, void *p)
* Just store the timestamp in hardirq context so we get it as
* close in time as possible to the actual event.
*/
- line->timestamp_ns = ktime_get_ns();
+ line->timestamp_ns = line_event_timestamp(line);
if (lr->num_lines != 1)
line->req_seqno = atomic_inc_return(>seqno);
@@ -673,7 +682,7 @@ static void debounce_work_func(struct work_struct *work)
memset(, 0, sizeof(le));
lr = line->req;
- le.timestamp_ns = ktime_get_ns();
+ le.timestamp_ns = line_event_timestamp(line);
le.offset = gpio_chip_hwgpio(line->desc);
line->line_seqno++;
le.line_seqno = line->line_seqno;
@@ -977,6 +986,9 @@ static void gpio_v2_line_config_flags_to_desc_flags(u64
flags,
flags & GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN);
assign_bit(FLAG_BIAS_DISABLE, flagsp,
flags & GPIO_V2_LINE_FLAG_BIAS_DISABLED);
+
+ assign_bit(FLAG_EVENT_CLOCK_REALTIME, flagsp,
+ flags & GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME);
}
static long linereq_get_values(struct linereq *lr, void __user *ip)
@@ -1940,6 +1952,9 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
if (test_bit(FLAG_EDGE_FALLING, >flags))
info->flags |= GPIO_V2_LINE_FLAG_EDGE_FALLING;
+ if (test_bit(FLAG_EVENT_CLOCK_REALTIME, >flags))
+ info->flags |= GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME;
+
debounce_period_us = READ_ONCE(desc->debounce_period_us);
if (debounce_period_us) {
info->attrs[num_attrs].id = GPIO_V2_LINE_ATTR_ID_DEBOUNCE;
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index b674b5bb980e..2f228ffe8320 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -116,6 +116,7 @@ struct gpio_desc {
#define FLAG_BIAS_DISABLE15/* GPIO has pull disabled */
#define FLAG_EDGE_RISING 16/* GPIO CDEV detects rising edge events
*/
#define FLAG_EDGE_FALLING17/* GPIO CDEV detects falling edge
events */
+#define FLAG_EVENT_CLOCK_REALTIME 18 /* GPIO CDEV reports REALTIME
timestamps in events */
/* Connection label */
const char *label;
diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index 2072c260f5d0..e4eb0b8c5cf9 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -65,6 +65,7 @@ struct gpiochip_info {
* @GPIO_V2_LINE_FLAG_BIAS_PULL_UP: line has pull-up bias enabled
* @GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN: line has pull-down bias enabled
* @GPIO_V2_LINE_FLAG_BIAS_DISABLED: line has bias
[PATCH v2 3/3] tools: gpio: add option to report wall-clock time to gpio-event-mon
Add support for selecting the realtime clock for events.
Signed-off-by: Kent Gibson
---
tools/gpio/gpio-event-mon.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/gpio/gpio-event-mon.c b/tools/gpio/gpio-event-mon.c
index 90c3155f05b1..cacd66ad7926 100644
--- a/tools/gpio/gpio-event-mon.c
+++ b/tools/gpio/gpio-event-mon.c
@@ -148,6 +148,7 @@ void print_usage(void)
" -s Set line as open source\n"
" -r Listen for rising edges\n"
" -f Listen for falling edges\n"
+ " -w Report the wall-clock time for events\n"
" -b Debounce the line with period n microseconds\n"
" [-c ]Do loops (optional, infinite loop if not
stated)\n"
" -? This helptext\n"
@@ -173,7 +174,7 @@ int main(int argc, char **argv)
memset(, 0, sizeof(config));
config.flags = GPIO_V2_LINE_FLAG_INPUT;
- while ((c = getopt(argc, argv, "c:n:o:b:dsrf?")) != -1) {
+ while ((c = getopt(argc, argv, "c:n:o:b:dsrfw?")) != -1) {
switch (c) {
case 'c':
loops = strtoul(optarg, NULL, 10);
@@ -204,6 +205,9 @@ int main(int argc, char **argv)
case 'f':
config.flags |= GPIO_V2_LINE_FLAG_EDGE_FALLING;
break;
+ case 'w':
+ config.flags |= GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME;
+ break;
case '?':
print_usage();
return -1;
--
2.28.0
Re: [PATCH 1/3] gpiolib: cdev: allow edge event timestamps to be configured as REALTIME
On Wed, Oct 14, 2020 at 02:27:38PM +0800, Kent Gibson wrote:
> Using CLOCK_REALTIME as the source for event timestamps is crucial for
> some specific applications, particularly those requiring timetamps
> relative to a PTP clock, so provide an option to switch the event
> timestamp source from the default CLOCK_MONOTONIC to CLOCK_REALTIME.
>
[snip]
>
> static void linereq_put_event(struct linereq *lr,
> @@ -535,6 +536,14 @@ static void linereq_put_event(struct linereq *lr,
> pr_debug_ratelimited("event FIFO is full - event dropped\n");
> }
>
> +static unsigned long line_event_timestamp(struct line *line)
> +{
> + if (test_bit(FLAG_EVENT_CLOCK_REALTIME, >desc->flags))
> + return ktime_get_real_ns();
> +
> + return ktime_get_ns();
> +
> +}
One minor hitch - that should be returning u64, not unsigned long,
or the time gets reduced to 32bit on 32bit platforms.
It's getting late though, so I'll send out an update tomorrow.
Cheers,
Kent.
[PATCH 1/3] gpiolib: cdev: allow edge event timestamps to be configured as REALTIME
Using CLOCK_REALTIME as the source for event timestamps is crucial for
some specific applications, particularly those requiring timetamps
relative to a PTP clock, so provide an option to switch the event
timestamp source from the default CLOCK_MONOTONIC to CLOCK_REALTIME.
Note that CLOCK_REALTIME was the default source clock for GPIO until
Linux 5.7 when it was changed to CLOCK_MONOTONIC due to issues with the
shifting of the realtime clock.
Providing this option maintains the CLOCK_MONOTONIC as the default,
while also providing a path forward for those dependent on the pre-5.7
behaviour.
Suggested-by: Jack Winch
Signed-off-by: Kent Gibson
---
drivers/gpio/gpiolib-cdev.c | 21 ++---
drivers/gpio/gpiolib.h | 1 +
include/uapi/linux/gpio.h | 12 +---
3 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 678de9264617..147626a78a92 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -515,6 +515,7 @@ struct linereq {
GPIO_V2_LINE_DIRECTION_FLAGS | \
GPIO_V2_LINE_DRIVE_FLAGS | \
GPIO_V2_LINE_EDGE_FLAGS | \
+GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME | \
GPIO_V2_LINE_BIAS_FLAGS)
static void linereq_put_event(struct linereq *lr,
@@ -535,6 +536,14 @@ static void linereq_put_event(struct linereq *lr,
pr_debug_ratelimited("event FIFO is full - event dropped\n");
}
+static unsigned long line_event_timestamp(struct line *line)
+{
+ if (test_bit(FLAG_EVENT_CLOCK_REALTIME, >desc->flags))
+ return ktime_get_real_ns();
+
+ return ktime_get_ns();
+
+}
static irqreturn_t edge_irq_thread(int irq, void *p)
{
struct line *line = p;
@@ -553,7 +562,7 @@ static irqreturn_t edge_irq_thread(int irq, void *p)
* which case we didn't get the timestamp from
* edge_irq_handler().
*/
- le.timestamp_ns = ktime_get_ns();
+ le.timestamp_ns = line_event_timestamp(line);
if (lr->num_lines != 1)
line->req_seqno = atomic_inc_return(>seqno);
}
@@ -598,7 +607,7 @@ static irqreturn_t edge_irq_handler(int irq, void *p)
* Just store the timestamp in hardirq context so we get it as
* close in time as possible to the actual event.
*/
- line->timestamp_ns = ktime_get_ns();
+ line->timestamp_ns = line_event_timestamp(line);
if (lr->num_lines != 1)
line->req_seqno = atomic_inc_return(>seqno);
@@ -673,7 +682,7 @@ static void debounce_work_func(struct work_struct *work)
memset(, 0, sizeof(le));
lr = line->req;
- le.timestamp_ns = ktime_get_ns();
+ le.timestamp_ns = line_event_timestamp(line);
le.offset = gpio_chip_hwgpio(line->desc);
line->line_seqno++;
le.line_seqno = line->line_seqno;
@@ -977,6 +986,9 @@ static void gpio_v2_line_config_flags_to_desc_flags(u64
flags,
flags & GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN);
assign_bit(FLAG_BIAS_DISABLE, flagsp,
flags & GPIO_V2_LINE_FLAG_BIAS_DISABLED);
+
+ assign_bit(FLAG_EVENT_CLOCK_REALTIME, flagsp,
+ flags & GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME);
}
static long linereq_get_values(struct linereq *lr, void __user *ip)
@@ -1940,6 +1952,9 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
if (test_bit(FLAG_EDGE_FALLING, >flags))
info->flags |= GPIO_V2_LINE_FLAG_EDGE_FALLING;
+ if (test_bit(FLAG_EVENT_CLOCK_REALTIME, >flags))
+ info->flags |= GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME;
+
debounce_period_us = READ_ONCE(desc->debounce_period_us);
if (debounce_period_us) {
info->attrs[num_attrs].id = GPIO_V2_LINE_ATTR_ID_DEBOUNCE;
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index b674b5bb980e..2f228ffe8320 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -116,6 +116,7 @@ struct gpio_desc {
#define FLAG_BIAS_DISABLE15/* GPIO has pull disabled */
#define FLAG_EDGE_RISING 16/* GPIO CDEV detects rising edge events
*/
#define FLAG_EDGE_FALLING17/* GPIO CDEV detects falling edge
events */
+#define FLAG_EVENT_CLOCK_REALTIME 18 /* GPIO CDEV reports REALTIME
timestamps in events */
/* Connection label */
const char *label;
diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index 2072c260f5d0..e4eb0b8c5cf9 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -65,6 +65,7 @@ struct gpiochip_info {
* @GPIO_V2_LINE_FLAG_BIAS_PULL_UP: line has pull-up bias enabled
* @GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN: line has pull-down bias enabled
* @GPIO_V2_LINE_FLAG_BIAS_DISABLED: line
[PATCH 3/3] tools: gpio: add option to report wall-clock time to gpio-event-mon
Add support for selecting the realtime clock for events.
Signed-off-by: Kent Gibson
---
tools/gpio/gpio-event-mon.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/gpio/gpio-event-mon.c b/tools/gpio/gpio-event-mon.c
index 90c3155f05b1..cacd66ad7926 100644
--- a/tools/gpio/gpio-event-mon.c
+++ b/tools/gpio/gpio-event-mon.c
@@ -148,6 +148,7 @@ void print_usage(void)
" -s Set line as open source\n"
" -r Listen for rising edges\n"
" -f Listen for falling edges\n"
+ " -w Report the wall-clock time for events\n"
" -b Debounce the line with period n microseconds\n"
" [-c ]Do loops (optional, infinite loop if not
stated)\n"
" -? This helptext\n"
@@ -173,7 +174,7 @@ int main(int argc, char **argv)
memset(, 0, sizeof(config));
config.flags = GPIO_V2_LINE_FLAG_INPUT;
- while ((c = getopt(argc, argv, "c:n:o:b:dsrf?")) != -1) {
+ while ((c = getopt(argc, argv, "c:n:o:b:dsrfw?")) != -1) {
switch (c) {
case 'c':
loops = strtoul(optarg, NULL, 10);
@@ -204,6 +205,9 @@ int main(int argc, char **argv)
case 'f':
config.flags |= GPIO_V2_LINE_FLAG_EDGE_FALLING;
break;
+ case 'w':
+ config.flags |= GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME;
+ break;
case '?':
print_usage();
return -1;
--
2.28.0
[PATCH 2/3] tools: gpio: add support for reporting realtime event clock to lsgpio
Add support for reporting if a line is configured to report realtime
timestamps in events.
Signed-off-by: Kent Gibson
---
tools/gpio/lsgpio.c | 4
1 file changed, 4 insertions(+)
diff --git a/tools/gpio/lsgpio.c b/tools/gpio/lsgpio.c
index 5a05a454d0c9..c61d061247e1 100644
--- a/tools/gpio/lsgpio.c
+++ b/tools/gpio/lsgpio.c
@@ -65,6 +65,10 @@ struct gpio_flag flagnames[] = {
.name = "bias-disabled",
.mask = GPIO_V2_LINE_FLAG_BIAS_DISABLED,
},
+ {
+ .name = "clock-realtime",
+ .mask = GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME,
+ },
};
static void print_attributes(struct gpio_v2_line_info *info)
--
2.28.0
[PATCH v2] gpiolib: cdev: document that line eflags are shared
The line.eflags field is shared so document this fact and highlight it
throughout using READ_ONCE() and WRITE_ONCE() accessors.
Also use a local copy of the eflags in edge_irq_thread() to ensure
consistent control flow even if eflags changes. This is only a defensive
measure as edge_irq_thread() is currently disabled when the eflags are
changed.
Signed-off-by: Kent Gibson
---
Changes for v2:
- fixed typo in commit comment.
drivers/gpio/gpiolib-cdev.c | 32 +---
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 192721f829a3..678de9264617 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -428,6 +428,12 @@ struct line {
*/
struct linereq *req;
unsigned int irq;
+ /*
+* eflags is set by edge_detector_setup(), edge_detector_stop() and
+* edge_detector_update(), which are themselves mutually exclusive,
+* and is accessed by edge_irq_thread() and debounce_work_func(),
+* which can both live with a slightly stale value.
+*/
u64 eflags;
/*
* timestamp_ns and req_seqno are accessed only by
@@ -534,6 +540,7 @@ static irqreturn_t edge_irq_thread(int irq, void *p)
struct line *line = p;
struct linereq *lr = line->req;
struct gpio_v2_line_event le;
+ u64 eflags;
/* Do not leak kernel stack to userspace */
memset(, 0, sizeof(le));
@@ -552,8 +559,9 @@ static irqreturn_t edge_irq_thread(int irq, void *p)
}
line->timestamp_ns = 0;
- if (line->eflags == (GPIO_V2_LINE_FLAG_EDGE_RISING |
-GPIO_V2_LINE_FLAG_EDGE_FALLING)) {
+ eflags = READ_ONCE(line->eflags);
+ if (eflags == (GPIO_V2_LINE_FLAG_EDGE_RISING |
+ GPIO_V2_LINE_FLAG_EDGE_FALLING)) {
int level = gpiod_get_value_cansleep(line->desc);
if (level)
@@ -562,10 +570,10 @@ static irqreturn_t edge_irq_thread(int irq, void *p)
else
/* Emit high-to-low event */
le.id = GPIO_V2_LINE_EVENT_FALLING_EDGE;
- } else if (line->eflags == GPIO_V2_LINE_FLAG_EDGE_RISING) {
+ } else if (eflags == GPIO_V2_LINE_FLAG_EDGE_RISING) {
/* Emit low-to-high event */
le.id = GPIO_V2_LINE_EVENT_RISING_EDGE;
- } else if (line->eflags == GPIO_V2_LINE_FLAG_EDGE_FALLING) {
+ } else if (eflags == GPIO_V2_LINE_FLAG_EDGE_FALLING) {
/* Emit high-to-low event */
le.id = GPIO_V2_LINE_EVENT_FALLING_EDGE;
} else {
@@ -634,6 +642,7 @@ static void debounce_work_func(struct work_struct *work)
struct line *line = container_of(work, struct line, work.work);
struct linereq *lr;
int level;
+ u64 eflags;
level = gpiod_get_raw_value_cansleep(line->desc);
if (level < 0) {
@@ -647,7 +656,8 @@ static void debounce_work_func(struct work_struct *work)
WRITE_ONCE(line->level, level);
/* -- edge detection -- */
- if (!line->eflags)
+ eflags = READ_ONCE(line->eflags);
+ if (!eflags)
return;
/* switch from physical level to logical - if they differ */
@@ -655,8 +665,8 @@ static void debounce_work_func(struct work_struct *work)
level = !level;
/* ignore edges that are not being monitored */
- if (((line->eflags == GPIO_V2_LINE_FLAG_EDGE_RISING) && !level) ||
- ((line->eflags == GPIO_V2_LINE_FLAG_EDGE_FALLING) && level))
+ if (((eflags == GPIO_V2_LINE_FLAG_EDGE_RISING) && !level) ||
+ ((eflags == GPIO_V2_LINE_FLAG_EDGE_FALLING) && level))
return;
/* Do not leak kernel stack to userspace */
@@ -755,7 +765,7 @@ static void edge_detector_stop(struct line *line)
cancel_delayed_work_sync(>work);
WRITE_ONCE(line->sw_debounced, 0);
- line->eflags = 0;
+ WRITE_ONCE(line->eflags, 0);
/* do not change line->level - see comment in debounced_value() */
}
@@ -774,7 +784,7 @@ static int edge_detector_setup(struct line *line,
if (ret)
return ret;
}
- line->eflags = eflags;
+ WRITE_ONCE(line->eflags, eflags);
if (gpio_v2_line_config_debounced(lc, line_idx)) {
debounce_period_us = gpio_v2_line_config_debounce_period(lc,
line_idx);
ret = debounce_setup(line, debounce_period_us);
@@ -817,13 +827,13 @@ static int edge_detector_update(struct line *line,
unsigned int debounce_period_us =
gpio_v2_line_config_debounce_period(lc, line_idx);
- if ((line->eflags == eflags) && !polarity_change &&
+ if ((READ_ONCE(line->e
[PATCH 0/3] gpiolib: cdev: allow edge event timestamps to be configured as REALTIME
This patch set adds the option to select CLOCK_REALTIME as the source clock for line events. The first patch is the core of the change, while the remaining two update the GPIO tools to make use of the new option. Kent Gibson (3): gpiolib: cdev: allow edge event timestamps to be configured as REALTIME tools: gpio: add support for reporting realtime event clock to lsgpio tools: gpio: add option to report wall-clock time to gpio-event-mon drivers/gpio/gpiolib-cdev.c | 21 ++--- drivers/gpio/gpiolib.h | 1 + include/uapi/linux/gpio.h | 12 +--- tools/gpio/gpio-event-mon.c | 6 +- tools/gpio/lsgpio.c | 4 5 files changed, 37 insertions(+), 7 deletions(-) -- 2.28.0
[PATCH] gpiolib: cdev: document that line eflags are shared
The line.elags field is shared so document this fact and highlight it
throughout using READ_ONCE() and WRITE_ONCE() accessors.
Also use a local copy of the eflags in edge_irq_thread() to ensure
consistent control flow even if eflags changes. This is only a defensive
measure as edge_irq_thread() is currently disabled when the eflags are
changed.
Signed-off-by: Kent Gibson
---
drivers/gpio/gpiolib-cdev.c | 32 +---
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 192721f829a3..678de9264617 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -428,6 +428,12 @@ struct line {
*/
struct linereq *req;
unsigned int irq;
+ /*
+* eflags is set by edge_detector_setup(), edge_detector_stop() and
+* edge_detector_update(), which are themselves mutually exclusive,
+* and is accessed by edge_irq_thread() and debounce_work_func(),
+* which can both live with a slightly stale value.
+*/
u64 eflags;
/*
* timestamp_ns and req_seqno are accessed only by
@@ -534,6 +540,7 @@ static irqreturn_t edge_irq_thread(int irq, void *p)
struct line *line = p;
struct linereq *lr = line->req;
struct gpio_v2_line_event le;
+ u64 eflags;
/* Do not leak kernel stack to userspace */
memset(, 0, sizeof(le));
@@ -552,8 +559,9 @@ static irqreturn_t edge_irq_thread(int irq, void *p)
}
line->timestamp_ns = 0;
- if (line->eflags == (GPIO_V2_LINE_FLAG_EDGE_RISING |
-GPIO_V2_LINE_FLAG_EDGE_FALLING)) {
+ eflags = READ_ONCE(line->eflags);
+ if (eflags == (GPIO_V2_LINE_FLAG_EDGE_RISING |
+ GPIO_V2_LINE_FLAG_EDGE_FALLING)) {
int level = gpiod_get_value_cansleep(line->desc);
if (level)
@@ -562,10 +570,10 @@ static irqreturn_t edge_irq_thread(int irq, void *p)
else
/* Emit high-to-low event */
le.id = GPIO_V2_LINE_EVENT_FALLING_EDGE;
- } else if (line->eflags == GPIO_V2_LINE_FLAG_EDGE_RISING) {
+ } else if (eflags == GPIO_V2_LINE_FLAG_EDGE_RISING) {
/* Emit low-to-high event */
le.id = GPIO_V2_LINE_EVENT_RISING_EDGE;
- } else if (line->eflags == GPIO_V2_LINE_FLAG_EDGE_FALLING) {
+ } else if (eflags == GPIO_V2_LINE_FLAG_EDGE_FALLING) {
/* Emit high-to-low event */
le.id = GPIO_V2_LINE_EVENT_FALLING_EDGE;
} else {
@@ -634,6 +642,7 @@ static void debounce_work_func(struct work_struct *work)
struct line *line = container_of(work, struct line, work.work);
struct linereq *lr;
int level;
+ u64 eflags;
level = gpiod_get_raw_value_cansleep(line->desc);
if (level < 0) {
@@ -647,7 +656,8 @@ static void debounce_work_func(struct work_struct *work)
WRITE_ONCE(line->level, level);
/* -- edge detection -- */
- if (!line->eflags)
+ eflags = READ_ONCE(line->eflags);
+ if (!eflags)
return;
/* switch from physical level to logical - if they differ */
@@ -655,8 +665,8 @@ static void debounce_work_func(struct work_struct *work)
level = !level;
/* ignore edges that are not being monitored */
- if (((line->eflags == GPIO_V2_LINE_FLAG_EDGE_RISING) && !level) ||
- ((line->eflags == GPIO_V2_LINE_FLAG_EDGE_FALLING) && level))
+ if (((eflags == GPIO_V2_LINE_FLAG_EDGE_RISING) && !level) ||
+ ((eflags == GPIO_V2_LINE_FLAG_EDGE_FALLING) && level))
return;
/* Do not leak kernel stack to userspace */
@@ -755,7 +765,7 @@ static void edge_detector_stop(struct line *line)
cancel_delayed_work_sync(>work);
WRITE_ONCE(line->sw_debounced, 0);
- line->eflags = 0;
+ WRITE_ONCE(line->eflags, 0);
/* do not change line->level - see comment in debounced_value() */
}
@@ -774,7 +784,7 @@ static int edge_detector_setup(struct line *line,
if (ret)
return ret;
}
- line->eflags = eflags;
+ WRITE_ONCE(line->eflags, eflags);
if (gpio_v2_line_config_debounced(lc, line_idx)) {
debounce_period_us = gpio_v2_line_config_debounce_period(lc,
line_idx);
ret = debounce_setup(line, debounce_period_us);
@@ -817,13 +827,13 @@ static int edge_detector_update(struct line *line,
unsigned int debounce_period_us =
gpio_v2_line_config_debounce_period(lc, line_idx);
- if ((line->eflags == eflags) && !polarity_change &&
+ if ((READ_ONCE(line->eflags) == eflags) && !polarity_change &&am
Re: [PATCH 0/5] gpio: uapi: documentation improvements
On Tue, Oct 13, 2020 at 03:21:47PM +0200, Linus Walleij wrote: > On Thu, Oct 8, 2020 at 5:46 PM Bartosz Golaszewski > wrote: > > On Mon, Oct 5, 2020 at 9:03 AM Kent Gibson wrote: > > > > > > I'm intending to add some GPIO chardev documentation to > > > Documentation/admin-guide/gpio/chardev.rst (or perhaps > > > Documentation/userspace-api/??), but that is taking longer than I'd like, > > > so in the meantime here is a collection of minor documentation tidy-ups > > > and improvements to the kernel-doc that I've made along the way. > > > Hopefully nothing controversial - mainly formatting improvements, > > > and a couple of minor wording changes. > > > For the entire series: > > > > Reviewed-by: Bartosz Golaszewski > > > > Linus: can you take them for v5.10 through your tree directly? > > I am waiting for Kent to respin them addressing Andy's comments > on patch 5/5 then they can go in as fixes I think. > I had replied to Andy's comments - I'm prefer with my version than his suggestion: "I'm not keen on that alternative as what it suggests is actually a pointer comparison, and even if the user realizes that they may instead use "strlen(label) == 0", when they shouldn't be assuming that a null terminator is present in the array. I avoided mentioning "string" and kept it in terms of the char array for the same reason." Cheers, Kent.
Re: [PATCH 5/5] gpio: uapi: clarify the meaning of 'empty' char arrays
On Mon, Oct 05, 2020 at 02:01:22PM +0300, Andy Shevchenko wrote: > On Mon, Oct 5, 2020 at 10:07 AM Kent Gibson wrote: > > > > Clarify that a char array containing a string is considered 'empty' if > > the first character is the null terminator. The remaining characters > > are not relevant to this determination. > > > * @label: a functional name for this GPIO chip, such as a product > > - * number, may be empty > > + * number, may be empty (i.e. label[0] == '\0') > > I would rather put it like > "...may be empty string (i.e. label == "")" > I'm not keen on that alternative as what it suggests is actually a pointer comparison, and even if the user realizes that they may instead use "strlen(label) == 0", when they shouldn't be assuming that a null terminator is present in the array. I avoided mentioning "string" and kept it in terms of the char array for the same reason. Cheers, Kent.
[PATCH 3/5] gpio: uapi: kernel-doc formatting improvements
Add kernel-doc formatting to all references to structs, enums, fields
and constants, and move deprecation warnings into the Note section of
the deprecated struct.
Replace 'OR:ed' with 'added', as the former looks odd.
Signed-off-by: Kent Gibson
---
The replacement of 'OR:ed' should strictly be in a separate patch, as
it isn't a formatting change, but as the same lines containing them were
being changed anyway it feels like overkill?
include/uapi/linux/gpio.h | 93 ---
1 file changed, 47 insertions(+), 46 deletions(-)
diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index 1fdb0e851f83..32dd18f238c3 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -110,17 +110,17 @@ enum gpio_v2_line_attr_id {
* struct gpio_v2_line_attribute - a configurable attribute of a line
* @id: attribute identifier with value from gpio_v2_line_attr_id
* @padding: reserved for future use and must be zero filled
- * @flags: if id is GPIO_V2_LINE_ATTR_ID_FLAGS, the flags for the GPIO
- * line, with values from enum gpio_v2_line_flag, such as
- * GPIO_V2_LINE_FLAG_ACTIVE_LOW, GPIO_V2_LINE_FLAG_OUTPUT etc, OR:ed
+ * @flags: if id is %GPIO_V2_LINE_ATTR_ID_FLAGS, the flags for the GPIO
+ * line, with values from gpio_v2_line_flag, such as
+ * %GPIO_V2_LINE_FLAG_ACTIVE_LOW, %GPIO_V2_LINE_FLAG_OUTPUT etc, added
* together. This overrides the default flags contained in the
* gpio_v2_line_config for the associated line.
- * @values: if id is GPIO_V2_LINE_ATTR_ID_OUTPUT_VALUES, a bitmap
+ * @values: if id is %GPIO_V2_LINE_ATTR_ID_OUTPUT_VALUES, a bitmap
* containing the values to which the lines will be set, with each bit
* number corresponding to the index into
* gpio_v2_line_request.offsets.
- * @debounce_period_us: if id is GPIO_V2_LINE_ATTR_ID_DEBOUNCE, the desired
- * debounce period, in microseconds
+ * @debounce_period_us: if id is %GPIO_V2_LINE_ATTR_ID_DEBOUNCE, the
+ * desired debounce period, in microseconds
*/
struct gpio_v2_line_attribute {
__u32 id;
@@ -147,12 +147,12 @@ struct gpio_v2_line_config_attribute {
/**
* struct gpio_v2_line_config - Configuration for GPIO lines
- * @flags: flags for the GPIO lines, with values from enum
- * gpio_v2_line_flag, such as GPIO_V2_LINE_FLAG_ACTIVE_LOW,
- * GPIO_V2_LINE_FLAG_OUTPUT etc, OR:ed together. This is the default for
+ * @flags: flags for the GPIO lines, with values from
+ * gpio_v2_line_flag, such as %GPIO_V2_LINE_FLAG_ACTIVE_LOW,
+ * %GPIO_V2_LINE_FLAG_OUTPUT etc, added together. This is the default for
* all requested lines but may be overridden for particular lines using
- * attrs.
- * @num_attrs: the number of attributes in attrs
+ * @attrs.
+ * @num_attrs: the number of attributes in @attrs
* @padding: reserved for future use and must be zero filled
* @attrs: the configuration attributes associated with the requested
* lines. Any attribute should only be associated with a particular line
@@ -175,17 +175,17 @@ struct gpio_v2_line_config {
* "my-bitbanged-relay"
* @config: requested configuration for the lines.
* @num_lines: number of lines requested in this request, i.e. the number
- * of valid fields in the GPIO_V2_LINES_MAX sized arrays, set to 1 to
+ * of valid fields in the %GPIO_V2_LINES_MAX sized arrays, set to 1 to
* request a single line
* @event_buffer_size: a suggested minimum number of line events that the
* kernel should buffer. This is only relevant if edge detection is
* enabled in the configuration. Note that this is only a suggested value
* and the kernel may allocate a larger buffer or cap the size of the
* buffer. If this field is zero then the buffer size defaults to a minimum
- * of num_lines*16.
+ * of @num_lines * 16.
* @padding: reserved for future use and must be zero filled
* @fd: if successful this field will contain a valid anonymous file handle
- * after a GPIO_GET_LINE_IOCTL operation, zero or negative value means
+ * after a %GPIO_GET_LINE_IOCTL operation, zero or negative value means
* error
*/
struct gpio_v2_line_request {
@@ -207,11 +207,12 @@ struct gpio_v2_line_request {
* @consumer: a functional name for the consumer of this GPIO line as set
* by whatever is using it, will be empty if there is no current user but
* may also be empty if the consumer doesn't set this up
- * @flags: flags for the GPIO line, such as GPIO_V2_LINE_FLAG_ACTIVE_LOW,
- * GPIO_V2_LINE_FLAG_OUTPUT etc, OR:ed together
* @offset: the local offset on this GPIO chip, fill this in when
* requesting the line information from the kernel
- * @num_attrs: the number of attributes in attrs
+ * @num_attrs: the number of attributes in @attrs
+ * @flags: flags for the GPIO lines, with values from
+ * gpio_v2_line_flag, such as %GPIO_V2_LINE_FLAG_ACTIVE_LOW,
+ * %GPIO_V2_LINE_FLAG_OUTPUT etc, added together.
* @attrs: the configuration attributes associated with the line
* @padding: reserved for
[PATCH 5/5] gpio: uapi: clarify the meaning of 'empty' char arrays
Clarify that a char array containing a string is considered 'empty' if
the first character is the null terminator. The remaining characters
are not relevant to this determination.
Signed-off-by: Kent Gibson
---
include/uapi/linux/gpio.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index ad3f56dd87ec..2072c260f5d0 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -26,7 +26,7 @@
* struct gpiochip_info - Information about a certain GPIO chip
* @name: the Linux kernel name of this GPIO chip
* @label: a functional name for this GPIO chip, such as a product
- * number, may be empty
+ * number, may be empty (i.e. label[0] == '\0')
* @lines: number of GPIO lines on this chip
*/
struct gpiochip_info {
@@ -203,7 +203,7 @@ struct gpio_v2_line_request {
* struct gpio_v2_line_info - Information about a certain GPIO line
* @name: the name of this GPIO line, such as the output pin of the line on
* the chip, a rail or a pin header name on a board, as specified by the
- * GPIO chip, may be empty
+ * GPIO chip, may be empty (i.e. name[0] == '\0')
* @consumer: a functional name for the consumer of this GPIO line as set
* by whatever is using it, will be empty if there is no current user but
* may also be empty if the consumer doesn't set this up
@@ -315,7 +315,7 @@ struct gpio_v2_line_event {
* @flags: various flags for this line
* @name: the name of this GPIO line, such as the output pin of the line on the
* chip, a rail or a pin header name on a board, as specified by the gpio
- * chip, may be empty
+ * chip, may be empty (i.e. name[0] == '\0')
* @consumer: a functional name for the consumer of this GPIO line as set by
* whatever is using it, will be empty if there is no current user but may
* also be empty if the consumer doesn't set this up
--
2.28.0
[PATCH 4/5] gpio: uapi: remove whitespace
Remove leading whitespace in ABI v1 comment.
Signed-off-by: Kent Gibson
---
include/uapi/linux/gpio.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index 32dd18f238c3..ad3f56dd87ec 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -292,7 +292,7 @@ struct gpio_v2_line_event {
};
/*
- * ABI v1
+ * ABI v1
*
* This version of the ABI is deprecated.
* Use the latest version of the ABI, defined above, instead.
--
2.28.0
[PATCH 2/5] gpio: uapi: comment consistency
Make debounce_period_us field documentation consistent with other fields
in the union.
Signed-off-by: Kent Gibson
---
include/uapi/linux/gpio.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index b0d5e7a1c693..1fdb0e851f83 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -98,7 +98,7 @@ struct gpio_v2_line_values {
* identifying which field of the attribute union is in use.
* @GPIO_V2_LINE_ATTR_ID_FLAGS: flags field is in use
* @GPIO_V2_LINE_ATTR_ID_OUTPUT_VALUES: values field is in use
- * @GPIO_V2_LINE_ATTR_ID_DEBOUNCE: debounce_period_us is in use
+ * @GPIO_V2_LINE_ATTR_ID_DEBOUNCE: debounce_period_us field is in use
*/
enum gpio_v2_line_attr_id {
GPIO_V2_LINE_ATTR_ID_FLAGS = 1,
--
2.28.0
[PATCH 0/5] gpio: uapi: documentation improvements
I'm intending to add some GPIO chardev documentation to Documentation/admin-guide/gpio/chardev.rst (or perhaps Documentation/userspace-api/??), but that is taking longer than I'd like, so in the meantime here is a collection of minor documentation tidy-ups and improvements to the kernel-doc that I've made along the way. Hopefully nothing controversial - mainly formatting improvements, and a couple of minor wording changes. Cheers, Kent. Kent Gibson (5): gpio: uapi: fix kernel-doc warnings gpio: uapi: comment consistency gpio: uapi: kernel-doc formatting improvements gpio: uapi: remove whitespace gpio: uapi: clarify the meaning of 'empty' char arrays include/uapi/linux/gpio.h | 106 +++--- 1 file changed, 54 insertions(+), 52 deletions(-) base-commit: 237d96164f2c2b33d0d5094192eb743e9e1b04ad -- 2.28.0
[PATCH 1/5] gpio: uapi: fix kernel-doc warnings
Fix kernel-doc warnings, specifically gpioline_info_changed.padding is
not documented and 'GPIO event types' describes defines, which are not
documented by kernel-doc.
Signed-off-by: Kent Gibson
---
include/uapi/linux/gpio.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index 07865c601099..b0d5e7a1c693 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -346,6 +346,7 @@ enum {
* @timestamp: estimate of time of status change occurrence, in nanoseconds
* @event_type: one of GPIOLINE_CHANGED_REQUESTED, GPIOLINE_CHANGED_RELEASED
* and GPIOLINE_CHANGED_CONFIG
+ * @padding: reserved for future use
*
* Note: struct gpioline_info embedded here has 32-bit alignment on its own,
* but it works fine with 64-bit alignment too. With its 72 byte size, we can
@@ -469,7 +470,7 @@ struct gpioevent_request {
int fd;
};
-/**
+/*
* GPIO event types
*/
#define GPIOEVENT_EVENT_RISING_EDGE 0x01
--
2.28.0
[PATCH] gpiolib: cdev: switch from kstrdup() to kstrndup()
Use kstrndup() to copy line labels from the userspace provided char
array, rather than ensuring the char array contains a null terminator
and using kstrdup().
Note that the length provided to kstrndup() still assumes that the char
array does contain a null terminator, so the maximum string length is one
less than the array. This is consistent with the previous behaviour.
Suggested-by: Andy Shevchenko
Signed-off-by: Kent Gibson
---
The change to kstrndup() was suggested by Andy as part of the review of
the uAPI v2. This patch is my initial interpretion of that suggestion,
applied to both the v2 case and the two corresponding v1 cases.
I have since realized that the consumer_label is copied back to userspace,
so in the existing code the consumer_label, which may have been modified,
is implicitly a return value. I doubt that is intentional or that it is
used as such, but strictly speaking this change may break the v1 ABI??
If so, it will be necessary to restore the setting of the last array entry
to '\0'.
Cheers,
Kent.
drivers/gpio/gpiolib-cdev.c | 27 +--
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 73386fcc252d..94733aab3224 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -307,11 +307,11 @@ static int linehandle_create(struct gpio_device *gdev,
void __user *ip)
lh->gdev = gdev;
get_device(>dev);
- /* Make sure this is terminated */
- handlereq.consumer_label[sizeof(handlereq.consumer_label)-1] = '\0';
- if (strlen(handlereq.consumer_label)) {
- lh->label = kstrdup(handlereq.consumer_label,
- GFP_KERNEL);
+ if (handlereq.consumer_label[0] != '\0') {
+ /* label is only initialized if consumer_label is set */
+ lh->label = kstrndup(handlereq.consumer_label,
+sizeof(handlereq.consumer_label) - 1,
+GFP_KERNEL);
if (!lh->label) {
ret = -ENOMEM;
goto out_free_lh;
@@ -1322,11 +1322,10 @@ static int linereq_create(struct gpio_device *gdev,
void __user *ip)
INIT_DELAYED_WORK(>lines[i].work, debounce_work_func);
}
- /* Make sure this is terminated */
- ulr.consumer[sizeof(ulr.consumer)-1] = '\0';
- if (strlen(ulr.consumer)) {
+ if (ulr.consumer[0] != '\0') {
/* label is only initialized if consumer is set */
- lr->label = kstrdup(ulr.consumer, GFP_KERNEL);
+ lr->label = kstrndup(ulr.consumer, sizeof(ulr.consumer) - 1,
+GFP_KERNEL);
if (!lr->label) {
ret = -ENOMEM;
goto out_free_linereq;
@@ -1711,11 +1710,11 @@ static int lineevent_create(struct gpio_device *gdev,
void __user *ip)
le->gdev = gdev;
get_device(>dev);
- /* Make sure this is terminated */
- eventreq.consumer_label[sizeof(eventreq.consumer_label)-1] = '\0';
- if (strlen(eventreq.consumer_label)) {
- le->label = kstrdup(eventreq.consumer_label,
- GFP_KERNEL);
+ if (eventreq.consumer_label[0] != '\0') {
+ /* label is only initialized if consumer_label is set */
+ le->label = kstrndup(eventreq.consumer_label,
+sizeof(eventreq.consumer_label) - 1,
+GFP_KERNEL);
if (!le->label) {
ret = -ENOMEM;
goto out_free_le;
base-commit: 237d96164f2c2b33d0d5094192eb743e9e1b04ad
--
2.28.0
Re: [PATCH 6/6] docs: gpio: add a new document to its index.rst
On Fri, Oct 02, 2020 at 07:49:50AM +0200, Mauro Carvalho Chehab wrote:
> There's now a new ReST file. Add it to the index.rst file.
>
> Fixes: ce7a2f77f976 ("docs: gpio: Add GPIO Aggregator documentation")
Shouldn't that be:
Fixes: fd1abe99e5fb ("Documentation: gpio: add documentation for gpio-mockup")
Cheers,
Kent.
> Signed-off-by: Mauro Carvalho Chehab
> ---
> Documentation/admin-guide/gpio/index.rst | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/admin-guide/gpio/index.rst
> b/Documentation/admin-guide/gpio/index.rst
> index ef2838638e96..7db367572f30 100644
> --- a/Documentation/admin-guide/gpio/index.rst
> +++ b/Documentation/admin-guide/gpio/index.rst
> @@ -9,6 +9,7 @@ gpio
>
> gpio-aggregator
> sysfs
> +gpio-mockup
>
> .. only:: subproject and html
>
> --
> 2.26.2
>
[PATCH v10 20/20] tools: gpio: add debounce support to gpio-event-mon
Add support for debouncing monitored lines to gpio-event-mon.
Signed-off-by: Kent Gibson
Reviewed-by: Andy Shevchenko
---
tools/gpio/gpio-event-mon.c | 20 +---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/tools/gpio/gpio-event-mon.c b/tools/gpio/gpio-event-mon.c
index 0c34f18f511c..90c3155f05b1 100644
--- a/tools/gpio/gpio-event-mon.c
+++ b/tools/gpio/gpio-event-mon.c
@@ -148,11 +148,12 @@ void print_usage(void)
" -s Set line as open source\n"
" -r Listen for rising edges\n"
" -f Listen for falling edges\n"
+ " -b Debounce the line with period n microseconds\n"
" [-c ]Do loops (optional, infinite loop if not
stated)\n"
" -? This helptext\n"
"\n"
"Example:\n"
- "gpio-event-mon -n gpiochip0 -o 4 -r -f\n"
+ "gpio-event-mon -n gpiochip0 -o 4 -r -f -b 1\n"
);
}
@@ -167,11 +168,12 @@ int main(int argc, char **argv)
unsigned int num_lines = 0;
unsigned int loops = 0;
struct gpio_v2_line_config config;
- int c;
+ int c, attr, i;
+ unsigned long debounce_period_us = 0;
memset(, 0, sizeof(config));
config.flags = GPIO_V2_LINE_FLAG_INPUT;
- while ((c = getopt(argc, argv, "c:n:o:dsrf?")) != -1) {
+ while ((c = getopt(argc, argv, "c:n:o:b:dsrf?")) != -1) {
switch (c) {
case 'c':
loops = strtoul(optarg, NULL, 10);
@@ -187,6 +189,9 @@ int main(int argc, char **argv)
lines[num_lines] = strtoul(optarg, NULL, 10);
num_lines++;
break;
+ case 'b':
+ debounce_period_us = strtoul(optarg, NULL, 10);
+ break;
case 'd':
config.flags |= GPIO_V2_LINE_FLAG_OPEN_DRAIN;
break;
@@ -205,6 +210,15 @@ int main(int argc, char **argv)
}
}
+ if (debounce_period_us) {
+ attr = config.num_attrs;
+ config.num_attrs++;
+ for (i = 0; i < num_lines; i++)
+ gpiotools_set_bit([attr].mask, i);
+ config.attrs[attr].attr.id = GPIO_V2_LINE_ATTR_ID_DEBOUNCE;
+ config.attrs[attr].attr.debounce_period_us = debounce_period_us;
+ }
+
if (!device_name || num_lines == 0) {
print_usage();
return -1;
--
2.28.0
[PATCH v10 18/20] tools: gpio: port gpio-event-mon to v2 uAPI
Port the gpio-event-mon tool to the latest GPIO uAPI.
Signed-off-by: Kent Gibson
Reviewed-by: Andy Shevchenko
---
tools/gpio/gpio-event-mon.c | 91 +++--
1 file changed, 47 insertions(+), 44 deletions(-)
diff --git a/tools/gpio/gpio-event-mon.c b/tools/gpio/gpio-event-mon.c
index 1a303a81aeef..b230af889f26 100644
--- a/tools/gpio/gpio-event-mon.c
+++ b/tools/gpio/gpio-event-mon.c
@@ -23,17 +23,16 @@
#include
#include
#include
+#include "gpio-utils.h"
int monitor_device(const char *device_name,
unsigned int line,
- uint32_t handleflags,
- uint32_t eventflags,
+ struct gpio_v2_line_config *config,
unsigned int loops)
{
- struct gpioevent_request req;
- struct gpiohandle_data data;
+ struct gpio_v2_line_values values;
char *chrdev_name;
- int fd;
+ int cfd, lfd;
int ret;
int i = 0;
@@ -41,44 +40,39 @@ int monitor_device(const char *device_name,
if (ret < 0)
return -ENOMEM;
- fd = open(chrdev_name, 0);
- if (fd == -1) {
+ cfd = open(chrdev_name, 0);
+ if (cfd == -1) {
ret = -errno;
fprintf(stderr, "Failed to open %s\n", chrdev_name);
goto exit_free_name;
}
- req.lineoffset = line;
- req.handleflags = handleflags;
- req.eventflags = eventflags;
- strcpy(req.consumer_label, "gpio-event-mon");
-
- ret = ioctl(fd, GPIO_GET_LINEEVENT_IOCTL, );
- if (ret == -1) {
- ret = -errno;
- fprintf(stderr, "Failed to issue GET EVENT "
- "IOCTL (%d)\n",
- ret);
- goto exit_close_error;
- }
+ ret = gpiotools_request_line(device_name, , 1, config,
+"gpio-event-mon");
+ if (ret < 0)
+ goto exit_device_close;
+ else
+ lfd = ret;
/* Read initial states */
- ret = ioctl(req.fd, GPIOHANDLE_GET_LINE_VALUES_IOCTL, );
- if (ret == -1) {
- ret = -errno;
- fprintf(stderr, "Failed to issue GPIOHANDLE GET LINE "
- "VALUES IOCTL (%d)\n",
+ values.mask = 1;
+ values.bits = 0;
+ ret = gpiotools_get_values(lfd, );
+ if (ret < 0) {
+ fprintf(stderr,
+ "Failed to issue GPIO LINE GET VALUES IOCTL (%d)\n",
ret);
- goto exit_close_error;
+ goto exit_line_close;
}
fprintf(stdout, "Monitoring line %d on %s\n", line, device_name);
- fprintf(stdout, "Initial line value: %d\n", data.values[0]);
+ fprintf(stdout, "Initial line value: %d\n",
+ gpiotools_test_bit(values.bits, 0));
while (1) {
- struct gpioevent_data event;
+ struct gpio_v2_line_event event;
- ret = read(req.fd, , sizeof(event));
+ ret = read(lfd, , sizeof(event));
if (ret == -1) {
if (errno == -EAGAIN) {
fprintf(stderr, "nothing available\n");
@@ -96,12 +90,14 @@ int monitor_device(const char *device_name,
ret = -EIO;
break;
}
- fprintf(stdout, "GPIO EVENT %llu: ", event.timestamp);
+ fprintf(stdout, "GPIO EVENT at %llu on line %d (%d|%d) ",
+ event.timestamp_ns, event.offset, event.line_seqno,
+ event.seqno);
switch (event.id) {
- case GPIOEVENT_EVENT_RISING_EDGE:
+ case GPIO_V2_LINE_EVENT_RISING_EDGE:
fprintf(stdout, "rising edge");
break;
- case GPIOEVENT_EVENT_FALLING_EDGE:
+ case GPIO_V2_LINE_EVENT_FALLING_EDGE:
fprintf(stdout, "falling edge");
break;
default:
@@ -114,8 +110,11 @@ int monitor_device(const char *device_name,
break;
}
-exit_close_error:
- if (close(fd) == -1)
+exit_line_close:
+ if (close(lfd) == -1)
+ perror("Failed to close line file");
+exit_device_close:
+ if (close(cfd) == -1)
perror("Failed to close GPIO character device file");
exit_free_name:
free(chrdev_name);
@@ -140,15 +139,20 @@ void print_usage(void)
);
}
+#define EDGE_FLAGS \
+ (GPIO_V2_LINE_FLAG_EDGE_RISING | \
+GPIO_V2_LINE_FLAG_EDGE_FALLING)
+
int main(int argc, char **argv)
{
const char *device_name = NU
[PATCH v10 19/20] tools: gpio: add multi-line monitoring to gpio-event-mon
Extend gpio-event-mon to support monitoring multiple lines.
This would require multiple lineevent requests to implement using uAPI v1,
but can be performed with a single line request using uAPI v2.
Signed-off-by: Kent Gibson
Reviewed-by: Andy Shevchenko
---
tools/gpio/gpio-event-mon.c | 45 -
1 file changed, 34 insertions(+), 11 deletions(-)
diff --git a/tools/gpio/gpio-event-mon.c b/tools/gpio/gpio-event-mon.c
index b230af889f26..0c34f18f511c 100644
--- a/tools/gpio/gpio-event-mon.c
+++ b/tools/gpio/gpio-event-mon.c
@@ -26,7 +26,8 @@
#include "gpio-utils.h"
int monitor_device(const char *device_name,
- unsigned int line,
+ unsigned int *lines,
+ unsigned int num_lines,
struct gpio_v2_line_config *config,
unsigned int loops)
{
@@ -47,7 +48,7 @@ int monitor_device(const char *device_name,
goto exit_free_name;
}
- ret = gpiotools_request_line(device_name, , 1, config,
+ ret = gpiotools_request_line(device_name, lines, num_lines, config,
"gpio-event-mon");
if (ret < 0)
goto exit_device_close;
@@ -55,8 +56,10 @@ int monitor_device(const char *device_name,
lfd = ret;
/* Read initial states */
- values.mask = 1;
+ values.mask = 0;
values.bits = 0;
+ for (i = 0; i < num_lines; i++)
+ gpiotools_set_bit(, i);
ret = gpiotools_get_values(lfd, );
if (ret < 0) {
fprintf(stderr,
@@ -65,9 +68,23 @@ int monitor_device(const char *device_name,
goto exit_line_close;
}
- fprintf(stdout, "Monitoring line %d on %s\n", line, device_name);
- fprintf(stdout, "Initial line value: %d\n",
- gpiotools_test_bit(values.bits, 0));
+ if (num_lines == 1) {
+ fprintf(stdout, "Monitoring line %d on %s\n", lines[0],
device_name);
+ fprintf(stdout, "Initial line value: %d\n",
+ gpiotools_test_bit(values.bits, 0));
+ } else {
+ fprintf(stdout, "Monitoring lines %d", lines[0]);
+ for (i = 1; i < num_lines - 1; i++)
+ fprintf(stdout, ", %d", lines[i]);
+ fprintf(stdout, " and %d on %s\n", lines[i], device_name);
+ fprintf(stdout, "Initial line values: %d",
+ gpiotools_test_bit(values.bits, 0));
+ for (i = 1; i < num_lines - 1; i++)
+ fprintf(stdout, ", %d",
+ gpiotools_test_bit(values.bits, i));
+ fprintf(stdout, " and %d\n",
+ gpiotools_test_bit(values.bits, i));
+ }
while (1) {
struct gpio_v2_line_event event;
@@ -126,7 +143,7 @@ void print_usage(void)
fprintf(stderr, "Usage: gpio-event-mon [options]...\n"
"Listen to events on GPIO lines, 0->1 1->0\n"
" -n Listen on GPIOs on a named device (must be
stated)\n"
- " -o Offset to monitor\n"
+ " -o Offset of line to monitor (may be repeated)\n"
" -d Set line as open drain\n"
" -s Set line as open source\n"
" -r Listen for rising edges\n"
@@ -146,7 +163,8 @@ void print_usage(void)
int main(int argc, char **argv)
{
const char *device_name = NULL;
- unsigned int line = -1;
+ unsigned int lines[GPIO_V2_LINES_MAX];
+ unsigned int num_lines = 0;
unsigned int loops = 0;
struct gpio_v2_line_config config;
int c;
@@ -162,7 +180,12 @@ int main(int argc, char **argv)
device_name = optarg;
break;
case 'o':
- line = strtoul(optarg, NULL, 10);
+ if (num_lines >= GPIO_V2_LINES_MAX) {
+ print_usage();
+ return -1;
+ }
+ lines[num_lines] = strtoul(optarg, NULL, 10);
+ num_lines++;
break;
case 'd':
config.flags |= GPIO_V2_LINE_FLAG_OPEN_DRAIN;
@@ -182,7 +205,7 @@ int main(int argc, char **argv)
}
}
- if (!device_name || line == -1) {
+ if (!device_name || num_lines == 0) {
print_usage();
return -1;
}
@@ -191,5 +214,5 @@ int main(int argc, char **argv)
"falling edges\n");
config.flags |= EDGE_FLAGS;
}
- return monitor_device(device_name, line, , loops);
+ return monitor_device(device_name, lines, num_lines, , loops);
}
--
2.28.0
[PATCH v10 12/20] gpiolib: cdev: support setting debounce
Add support for setting debounce on a line via the GPIO uAPI.
Where debounce is not supported by hardware, a software debounce is
provided.
The implementation of the software debouncer waits for the line to be
stable for the debounce period before determining if a level change,
and a corresponding edge event, has occurred. This provides maximum
protection against glitches, but also introduces a debounce_period
latency to edge events.
The software debouncer is integrated with the edge detection as it
utilises the line interrupt, and integration is simpler than getting
the two to interwork. Where software debounce AND edge detection is
required, the debouncer provides both.
Signed-off-by: Kent Gibson
---
Changes for v10:
- as per cover letter
Changes for v6:
- as per cover letter
Changes for v5:
- as per cover letter
Changes for v4:
- fix handling of mask in line_get_values
Changes for v3:
- only GPIO_V2 field renaming
Changes for v2:
- improve documentation on fields shared by threads.
- use READ_ONCE/WRITE_ONCE for shared fields rather than atomic_t
which was overkill.
drivers/gpio/gpiolib-cdev.c | 247 ++--
drivers/gpio/gpiolib.c | 3 +
drivers/gpio/gpiolib.h | 4 +
3 files changed, 244 insertions(+), 10 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 25536aae3e18..73386fcc252d 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -6,6 +6,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -22,6 +23,7 @@
#include
#include
#include
+#include
#include
#include "gpiolib.h"
@@ -415,6 +417,9 @@ static int linehandle_create(struct gpio_device *gdev, void
__user *ip)
* events for the corresponding line request. This is drawn from the @req.
* @line_seqno: the seqno for the current edge event in the sequence of
* events for this line.
+ * @work: the worker that implements software debouncing
+ * @sw_debounced: flag indicating if the software debouncer is active
+ * @level: the current debounced physical level of the line
*/
struct line {
struct gpio_desc *desc;
@@ -431,7 +436,28 @@ struct line {
*/
u64 timestamp_ns;
u32 req_seqno;
+ /*
+* line_seqno is accessed by either edge_irq_thread() or
+* debounce_work_func(), which are themselves mutually exclusive,
+* so no additional protection is necessary.
+*/
u32 line_seqno;
+ /*
+* -- debouncer specific fields --
+*/
+ struct delayed_work work;
+ /*
+* sw_debounce is accessed by linereq_set_config(), which is the
+* only setter, and linereq_get_values(), which can live with a
+* slightly stale value.
+*/
+ unsigned int sw_debounced;
+ /*
+* level is accessed by debounce_work_func(), which is the only
+* setter, and linereq_get_values() which can live with a slightly
+* stale value.
+*/
+ unsigned int level;
};
/**
@@ -572,6 +598,154 @@ static irqreturn_t edge_irq_handler(int irq, void *p)
return IRQ_WAKE_THREAD;
}
+/*
+ * returns the current debounced logical value.
+ */
+static bool debounced_value(struct line *line)
+{
+ bool value;
+
+ /*
+* minor race - debouncer may be stopped here, so edge_detector_stop()
+* must leave the value unchanged so the following will read the level
+* from when the debouncer was last running.
+*/
+ value = READ_ONCE(line->level);
+
+ if (test_bit(FLAG_ACTIVE_LOW, >desc->flags))
+ value = !value;
+
+ return value;
+}
+
+static irqreturn_t debounce_irq_handler(int irq, void *p)
+{
+ struct line *line = p;
+
+ mod_delayed_work(system_wq, >work,
+ usecs_to_jiffies(READ_ONCE(line->desc->debounce_period_us)));
+
+ return IRQ_HANDLED;
+}
+
+static void debounce_work_func(struct work_struct *work)
+{
+ struct gpio_v2_line_event le;
+ struct line *line = container_of(work, struct line, work.work);
+ struct linereq *lr;
+ int level;
+
+ level = gpiod_get_raw_value_cansleep(line->desc);
+ if (level < 0) {
+ pr_debug_ratelimited("debouncer failed to read line value\n");
+ return;
+ }
+
+ if (READ_ONCE(line->level) == level)
+ return;
+
+ WRITE_ONCE(line->level, level);
+
+ /* -- edge detection -- */
+ if (!line->eflags)
+ return;
+
+ /* switch from physical level to logical - if they differ */
+ if (test_bit(FLAG_ACTIVE_LOW, >desc->flags))
+ level = !level;
+
+ /* ignore edges that are not being monitored */
+ if (((line->eflags == GPIO_V2_LINE_FLAG_EDGE_RISING) && !level) ||
+ ((line->eflags == GPIO_V2_L
[PATCH v10 14/20] tools: gpio: port lsgpio to v2 uAPI
Port the lsgpio tool to the latest GPIO uAPI.
Signed-off-by: Kent Gibson
Reviewed-by: Andy Shevchenko
---
tools/gpio/lsgpio.c | 60 -
1 file changed, 38 insertions(+), 22 deletions(-)
diff --git a/tools/gpio/lsgpio.c b/tools/gpio/lsgpio.c
index b08d7a5e779b..5a05a454d0c9 100644
--- a/tools/gpio/lsgpio.c
+++ b/tools/gpio/lsgpio.c
@@ -25,57 +25,73 @@
struct gpio_flag {
char *name;
- unsigned long mask;
+ unsigned long long mask;
};
struct gpio_flag flagnames[] = {
{
- .name = "kernel",
- .mask = GPIOLINE_FLAG_KERNEL,
+ .name = "used",
+ .mask = GPIO_V2_LINE_FLAG_USED,
+ },
+ {
+ .name = "input",
+ .mask = GPIO_V2_LINE_FLAG_INPUT,
},
{
.name = "output",
- .mask = GPIOLINE_FLAG_IS_OUT,
+ .mask = GPIO_V2_LINE_FLAG_OUTPUT,
},
{
.name = "active-low",
- .mask = GPIOLINE_FLAG_ACTIVE_LOW,
+ .mask = GPIO_V2_LINE_FLAG_ACTIVE_LOW,
},
{
.name = "open-drain",
- .mask = GPIOLINE_FLAG_OPEN_DRAIN,
+ .mask = GPIO_V2_LINE_FLAG_OPEN_DRAIN,
},
{
.name = "open-source",
- .mask = GPIOLINE_FLAG_OPEN_SOURCE,
+ .mask = GPIO_V2_LINE_FLAG_OPEN_SOURCE,
},
{
.name = "pull-up",
- .mask = GPIOLINE_FLAG_BIAS_PULL_UP,
+ .mask = GPIO_V2_LINE_FLAG_BIAS_PULL_UP,
},
{
.name = "pull-down",
- .mask = GPIOLINE_FLAG_BIAS_PULL_DOWN,
+ .mask = GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN,
},
{
.name = "bias-disabled",
- .mask = GPIOLINE_FLAG_BIAS_DISABLE,
+ .mask = GPIO_V2_LINE_FLAG_BIAS_DISABLED,
},
};
-void print_flags(unsigned long flags)
+static void print_attributes(struct gpio_v2_line_info *info)
{
int i;
- int printed = 0;
+ const char *field_format = "%s";
for (i = 0; i < ARRAY_SIZE(flagnames); i++) {
- if (flags & flagnames[i].mask) {
- if (printed)
- fprintf(stdout, " ");
- fprintf(stdout, "%s", flagnames[i].name);
- printed++;
+ if (info->flags & flagnames[i].mask) {
+ fprintf(stdout, field_format, flagnames[i].name);
+ field_format = ", %s";
}
}
+
+ if ((info->flags & GPIO_V2_LINE_FLAG_EDGE_RISING) &&
+ (info->flags & GPIO_V2_LINE_FLAG_EDGE_FALLING))
+ fprintf(stdout, field_format, "both-edges");
+ else if (info->flags & GPIO_V2_LINE_FLAG_EDGE_RISING)
+ fprintf(stdout, field_format, "rising-edge");
+ else if (info->flags & GPIO_V2_LINE_FLAG_EDGE_FALLING)
+ fprintf(stdout, field_format, "falling-edge");
+
+ for (i = 0; i < info->num_attrs; i++) {
+ if (info->attrs[i].id == GPIO_V2_LINE_ATTR_ID_DEBOUNCE)
+ fprintf(stdout, ", debounce_period=%dusec",
+ info->attrs[0].debounce_period_us);
+ }
}
int list_device(const char *device_name)
@@ -109,18 +125,18 @@ int list_device(const char *device_name)
/* Loop over the lines and print info */
for (i = 0; i < cinfo.lines; i++) {
- struct gpioline_info linfo;
+ struct gpio_v2_line_info linfo;
memset(, 0, sizeof(linfo));
- linfo.line_offset = i;
+ linfo.offset = i;
- ret = ioctl(fd, GPIO_GET_LINEINFO_IOCTL, );
+ ret = ioctl(fd, GPIO_V2_GET_LINEINFO_IOCTL, );
if (ret == -1) {
ret = -errno;
perror("Failed to issue LINEINFO IOCTL\n");
goto exit_close_error;
}
- fprintf(stdout, "\tline %2d:", linfo.line_offset);
+ fprintf(stdout, "\tline %2d:", linfo.offset);
if (linfo.name[0])
fprintf(stdout, " \"%s\"", linfo.name);
else
@@ -131,7 +147,7 @@ int list_device(const char *device_name)
fprintf(stdout, " unused");
if (linfo.flags) {
fprintf(stdout, " [");
- print_flags(linfo.flags);
+ print_attributes();
fprintf(stdout, "]");
}
fprintf(stdout, "\n");
--
2.28.0
[PATCH v10 17/20] tools: gpio: port gpio-hammer to v2 uAPI
Port the gpio-hammer tool to the latest GPIO uAPI.
Signed-off-by: Kent Gibson
Reviewed-by: Andy Shevchenko
---
tools/gpio/gpio-hammer.c | 32 +---
tools/gpio/gpio-utils.c | 164 ---
tools/gpio/gpio-utils.h | 46 ++-
3 files changed, 197 insertions(+), 45 deletions(-)
diff --git a/tools/gpio/gpio-hammer.c b/tools/gpio/gpio-hammer.c
index a2c7577fad5c..54fdf59dd320 100644
--- a/tools/gpio/gpio-hammer.c
+++ b/tools/gpio/gpio-hammer.c
@@ -25,23 +25,30 @@
int hammer_device(const char *device_name, unsigned int *lines, int num_lines,
unsigned int loops)
{
- struct gpiohandle_data data;
+ struct gpio_v2_line_values values;
+ struct gpio_v2_line_config config;
char swirr[] = "-\\|/";
int fd;
int ret;
int i, j;
unsigned int iteration = 0;
- memset(, 0, sizeof(data.values));
- ret = gpiotools_request_linehandle(device_name, lines, num_lines,
- GPIOHANDLE_REQUEST_OUTPUT, ,
- "gpio-hammer");
+ memset(, 0, sizeof(config));
+ config.flags = GPIO_V2_LINE_FLAG_OUTPUT;
+
+ ret = gpiotools_request_line(device_name, lines, num_lines,
+, "gpio-hammer");
if (ret < 0)
goto exit_error;
else
fd = ret;
- ret = gpiotools_get_values(fd, );
+ values.mask = 0;
+ values.bits = 0;
+ for (i = 0; i < num_lines; i++)
+ gpiotools_set_bit(, i);
+
+ ret = gpiotools_get_values(fd, );
if (ret < 0)
goto exit_close_error;
@@ -53,7 +60,7 @@ int hammer_device(const char *device_name, unsigned int
*lines, int num_lines,
}
fprintf(stdout, "] on %s, initial states: [", device_name);
for (i = 0; i < num_lines; i++) {
- fprintf(stdout, "%d", data.values[i]);
+ fprintf(stdout, "%d", gpiotools_test_bit(values.bits, i));
if (i != (num_lines - 1))
fprintf(stdout, ", ");
}
@@ -64,14 +71,14 @@ int hammer_device(const char *device_name, unsigned int
*lines, int num_lines,
while (1) {
/* Invert all lines so we blink */
for (i = 0; i < num_lines; i++)
- data.values[i] = !data.values[i];
+ gpiotools_change_bit(, i);
- ret = gpiotools_set_values(fd, );
+ ret = gpiotools_set_values(fd, );
if (ret < 0)
goto exit_close_error;
/* Re-read values to get status */
- ret = gpiotools_get_values(fd, );
+ ret = gpiotools_get_values(fd, );
if (ret < 0)
goto exit_close_error;
@@ -82,7 +89,8 @@ int hammer_device(const char *device_name, unsigned int
*lines, int num_lines,
fprintf(stdout, "[");
for (i = 0; i < num_lines; i++) {
- fprintf(stdout, "%d: %d", lines[i], data.values[i]);
+ fprintf(stdout, "%d: %d", lines[i],
+ gpiotools_test_bit(values.bits, i));
if (i != (num_lines - 1))
fprintf(stdout, ", ");
}
@@ -97,7 +105,7 @@ int hammer_device(const char *device_name, unsigned int
*lines, int num_lines,
ret = 0;
exit_close_error:
- gpiotools_release_linehandle(fd);
+ gpiotools_release_line(fd);
exit_error:
return ret;
}
diff --git a/tools/gpio/gpio-utils.c b/tools/gpio/gpio-utils.c
index d527980bcb94..37187e056c8b 100644
--- a/tools/gpio/gpio-utils.c
+++ b/tools/gpio/gpio-utils.c
@@ -100,20 +100,87 @@ int gpiotools_request_linehandle(const char *device_name,
unsigned int *lines,
free(chrdev_name);
return ret < 0 ? ret : req.fd;
}
+
+/**
+ * gpiotools_request_line() - request gpio lines in a gpiochip
+ * @device_name: The name of gpiochip without prefix "/dev/",
+ * such as "gpiochip0"
+ * @lines: An array desired lines, specified by offset
+ * index for the associated GPIO device.
+ * @num_lines: The number of lines to request.
+ * @config:The new config for requested gpio. Reference
+ * "linux/gpio.h" for config details.
+ * @consumer: The name of consumer, such as "sysfs",
+ * "powerkey". This is useful for other users to
+ * know who is using.
+ *
+ * Request gpio lines through the ioctl provided by chardev. User
+ * could call gpiotools_set_values() and gpiotools_get_val
[PATCH v10 15/20] tools: gpio: port gpio-watch to v2 uAPI
Port the gpio-watch tool to the latest GPIO uAPI.
Signed-off-by: Kent Gibson
Reviewed-by: Andy Shevchenko
---
tools/gpio/gpio-watch.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/gpio/gpio-watch.c b/tools/gpio/gpio-watch.c
index 5cea24fddfa7..f229ec62301b 100644
--- a/tools/gpio/gpio-watch.c
+++ b/tools/gpio/gpio-watch.c
@@ -21,8 +21,8 @@
int main(int argc, char **argv)
{
- struct gpioline_info_changed chg;
- struct gpioline_info req;
+ struct gpio_v2_line_info_changed chg;
+ struct gpio_v2_line_info req;
struct pollfd pfd;
int fd, i, j, ret;
char *event, *end;
@@ -40,11 +40,11 @@ int main(int argc, char **argv)
for (i = 0, j = 2; i < argc - 2; i++, j++) {
memset(, 0, sizeof(req));
- req.line_offset = strtoul(argv[j], , 0);
+ req.offset = strtoul(argv[j], , 0);
if (*end != '\0')
goto err_usage;
- ret = ioctl(fd, GPIO_GET_LINEINFO_WATCH_IOCTL, );
+ ret = ioctl(fd, GPIO_V2_GET_LINEINFO_WATCH_IOCTL, );
if (ret) {
perror("unable to set up line watch");
return EXIT_FAILURE;
@@ -71,13 +71,13 @@ int main(int argc, char **argv)
}
switch (chg.event_type) {
- case GPIOLINE_CHANGED_REQUESTED:
+ case GPIO_V2_LINE_CHANGED_REQUESTED:
event = "requested";
break;
- case GPIOLINE_CHANGED_RELEASED:
+ case GPIO_V2_LINE_CHANGED_RELEASED:
event = "released";
break;
- case GPIOLINE_CHANGED_CONFIG:
+ case GPIO_V2_LINE_CHANGED_CONFIG:
event = "config changed";
break;
default:
@@ -87,7 +87,7 @@ int main(int argc, char **argv)
}
printf("line %u: %s at %llu\n",
- chg.info.line_offset, event, chg.timestamp);
+ chg.info.offset, event, chg.timestamp_ns);
}
}
--
2.28.0
[PATCH v10 16/20] tools: gpio: rename nlines to num_lines
Rename nlines to num_lines to be consistent with other usage for fields
describing the number of entries in an array.
Signed-off-by: Kent Gibson
Reviewed-by: Andy Shevchenko
---
tools/gpio/gpio-hammer.c | 26 +-
tools/gpio/gpio-utils.c | 20 ++--
tools/gpio/gpio-utils.h | 6 +++---
3 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/tools/gpio/gpio-hammer.c b/tools/gpio/gpio-hammer.c
index 9fd926e8cb52..a2c7577fad5c 100644
--- a/tools/gpio/gpio-hammer.c
+++ b/tools/gpio/gpio-hammer.c
@@ -22,7 +22,7 @@
#include
#include "gpio-utils.h"
-int hammer_device(const char *device_name, unsigned int *lines, int nlines,
+int hammer_device(const char *device_name, unsigned int *lines, int num_lines,
unsigned int loops)
{
struct gpiohandle_data data;
@@ -33,7 +33,7 @@ int hammer_device(const char *device_name, unsigned int
*lines, int nlines,
unsigned int iteration = 0;
memset(, 0, sizeof(data.values));
- ret = gpiotools_request_linehandle(device_name, lines, nlines,
+ ret = gpiotools_request_linehandle(device_name, lines, num_lines,
GPIOHANDLE_REQUEST_OUTPUT, ,
"gpio-hammer");
if (ret < 0)
@@ -46,15 +46,15 @@ int hammer_device(const char *device_name, unsigned int
*lines, int nlines,
goto exit_close_error;
fprintf(stdout, "Hammer lines [");
- for (i = 0; i < nlines; i++) {
+ for (i = 0; i < num_lines; i++) {
fprintf(stdout, "%d", lines[i]);
- if (i != (nlines - 1))
+ if (i != (num_lines - 1))
fprintf(stdout, ", ");
}
fprintf(stdout, "] on %s, initial states: [", device_name);
- for (i = 0; i < nlines; i++) {
+ for (i = 0; i < num_lines; i++) {
fprintf(stdout, "%d", data.values[i]);
- if (i != (nlines - 1))
+ if (i != (num_lines - 1))
fprintf(stdout, ", ");
}
fprintf(stdout, "]\n");
@@ -63,7 +63,7 @@ int hammer_device(const char *device_name, unsigned int
*lines, int nlines,
j = 0;
while (1) {
/* Invert all lines so we blink */
- for (i = 0; i < nlines; i++)
+ for (i = 0; i < num_lines; i++)
data.values[i] = !data.values[i];
ret = gpiotools_set_values(fd, );
@@ -81,9 +81,9 @@ int hammer_device(const char *device_name, unsigned int
*lines, int nlines,
j = 0;
fprintf(stdout, "[");
- for (i = 0; i < nlines; i++) {
+ for (i = 0; i < num_lines; i++) {
fprintf(stdout, "%d: %d", lines[i], data.values[i]);
- if (i != (nlines - 1))
+ if (i != (num_lines - 1))
fprintf(stdout, ", ");
}
fprintf(stdout, "]\r");
@@ -121,7 +121,7 @@ int main(int argc, char **argv)
const char *device_name = NULL;
unsigned int lines[GPIOHANDLES_MAX];
unsigned int loops = 0;
- int nlines;
+ int num_lines;
int c;
int i;
@@ -158,11 +158,11 @@ int main(int argc, char **argv)
return -1;
}
- nlines = i;
+ num_lines = i;
- if (!device_name || !nlines) {
+ if (!device_name || !num_lines) {
print_usage();
return -1;
}
- return hammer_device(device_name, lines, nlines, loops);
+ return hammer_device(device_name, lines, num_lines, loops);
}
diff --git a/tools/gpio/gpio-utils.c b/tools/gpio/gpio-utils.c
index 16a5d9cb9da2..d527980bcb94 100644
--- a/tools/gpio/gpio-utils.c
+++ b/tools/gpio/gpio-utils.c
@@ -38,7 +38,7 @@
* such as "gpiochip0"
* @lines: An array desired lines, specified by offset
* index for the associated GPIO device.
- * @nline: The number of lines to request.
+ * @num_lines: The number of lines to request.
* @flag: The new flag for requsted gpio. Reference
* "linux/gpio.h" for the meaning of flag.
* @data: Default value will be set to gpio when flag is
@@ -56,7 +56,7 @@
* On failure return the errno.
*/
int gpiotools_request_linehandle(const char *device_name, unsigned int *lines,
-unsigned int nlines, unsigned int flag,
+unsigned int num_lines, unsigned int flag,
struct gpiohandle_data *data,
[PATCH v10 13/20] gpio: uapi: document uAPI v1 as deprecated
Update uAPI documentation to deprecate v1 structs and ioctls.
Signed-off-by: Kent Gibson
---
include/uapi/linux/gpio.h | 26 ++
1 file changed, 26 insertions(+)
diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index 5904f49399de..07865c601099 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -292,6 +292,9 @@ struct gpio_v2_line_event {
/*
* ABI v1
+ *
+ * This version of the ABI is deprecated.
+ * Use the latest version of the ABI, defined above, instead.
*/
/* Informational flags */
@@ -315,6 +318,9 @@ struct gpio_v2_line_event {
* @consumer: a functional name for the consumer of this GPIO line as set by
* whatever is using it, will be empty if there is no current user but may
* also be empty if the consumer doesn't set this up
+ *
+ * This struct is part of ABI v1 and is deprecated.
+ * Use struct gpio_v2_line_info instead.
*/
struct gpioline_info {
__u32 line_offset;
@@ -346,6 +352,9 @@ enum {
* guarantee there are no implicit holes between it and subsequent members.
* The 20-byte padding at the end makes sure we don't add any implicit padding
* at the end of the structure on 64-bit architectures.
+ *
+ * This struct is part of ABI v1 and is deprecated.
+ * Use struct gpio_v2_line_info_changed instead.
*/
struct gpioline_info_changed {
struct gpioline_info info;
@@ -385,6 +394,9 @@ struct gpioline_info_changed {
* @fd: if successful this field will contain a valid anonymous file handle
* after a GPIO_GET_LINEHANDLE_IOCTL operation, zero or negative value
* means error
+ *
+ * This struct is part of ABI v1 and is deprecated.
+ * Use struct gpio_v2_line_request instead.
*/
struct gpiohandle_request {
__u32 lineoffsets[GPIOHANDLES_MAX];
@@ -404,6 +416,9 @@ struct gpiohandle_request {
* this specifies the default output value, should be 0 (low) or
* 1 (high), anything else than 0 or 1 will be interpreted as 1 (high)
* @padding: reserved for future use and should be zero filled
+ *
+ * This struct is part of ABI v1 and is deprecated.
+ * Use struct gpio_v2_line_config instead.
*/
struct gpiohandle_config {
__u32 flags;
@@ -416,6 +431,9 @@ struct gpiohandle_config {
* @values: when getting the state of lines this contains the current
* state of a line, when setting the state of lines these should contain
* the desired target state
+ *
+ * This struct is part of ABI v1 and is deprecated.
+ * Use struct gpio_v2_line_values instead.
*/
struct gpiohandle_data {
__u8 values[GPIOHANDLES_MAX];
@@ -439,6 +457,9 @@ struct gpiohandle_data {
* @fd: if successful this field will contain a valid anonymous file handle
* after a GPIO_GET_LINEEVENT_IOCTL operation, zero or negative value
* means error
+ *
+ * This struct is part of ABI v1 and is deprecated.
+ * Use struct gpio_v2_line_request instead.
*/
struct gpioevent_request {
__u32 lineoffset;
@@ -458,6 +479,9 @@ struct gpioevent_request {
* struct gpioevent_data - The actual event being pushed to userspace
* @timestamp: best estimate of time of event occurrence, in nanoseconds
* @id: event identifier
+ *
+ * This struct is part of ABI v1 and is deprecated.
+ * Use struct gpio_v2_line_event instead.
*/
struct gpioevent_data {
__u64 timestamp;
@@ -482,6 +506,8 @@ struct gpioevent_data {
/*
* v1 ioctl()s
+ *
+ * These ioctl()s are deprecated. Use the v2 equivalent instead.
*/
#define GPIO_GET_LINEINFO_IOCTL _IOWR(0xB4, 0x02, struct gpioline_info)
#define GPIO_GET_LINEHANDLE_IOCTL _IOWR(0xB4, 0x03, struct gpiohandle_request)
--
2.28.0
[PATCH v10 11/20] gpiolib: cdev: support GPIO_V2_LINE_SET_VALUES_IOCTL
Add support for the GPIO_V2_LINE_SET_VALUES_IOCTL.
Signed-off-by: Kent Gibson
---
drivers/gpio/gpiolib-cdev.c | 61 +
1 file changed, 61 insertions(+)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 608cdbd1d579..25536aae3e18 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -816,6 +816,65 @@ static long linereq_get_values(struct linereq *lr, void
__user *ip)
return 0;
}
+static long linereq_set_values_unlocked(struct linereq *lr,
+ struct gpio_v2_line_values *lv)
+{
+ DECLARE_BITMAP(vals, GPIO_V2_LINES_MAX);
+ struct gpio_desc **descs;
+ unsigned int i, didx, num_set;
+ int ret;
+
+ bitmap_zero(vals, GPIO_V2_LINES_MAX);
+ for (num_set = 0, i = 0; i < lr->num_lines; i++) {
+ if (lv->mask & BIT_ULL(i)) {
+ if (!test_bit(FLAG_IS_OUT, >lines[i].desc->flags))
+ return -EPERM;
+ if (lv->bits & BIT_ULL(i))
+ __set_bit(num_set, vals);
+ num_set++;
+ descs = >lines[i].desc;
+ }
+ }
+ if (num_set == 0)
+ return -EINVAL;
+
+ if (num_set != 1) {
+ /* build compacted desc array and values */
+ descs = kmalloc_array(num_set, sizeof(*descs), GFP_KERNEL);
+ if (!descs)
+ return -ENOMEM;
+ for (didx = 0, i = 0; i < lr->num_lines; i++) {
+ if (lv->mask & BIT_ULL(i)) {
+ descs[didx] = lr->lines[i].desc;
+ didx++;
+ }
+ }
+ }
+ ret = gpiod_set_array_value_complex(false, true, num_set,
+ descs, NULL, vals);
+
+ if (num_set != 1)
+ kfree(descs);
+ return ret;
+}
+
+static long linereq_set_values(struct linereq *lr, void __user *ip)
+{
+ struct gpio_v2_line_values lv;
+ int ret;
+
+ if (copy_from_user(, ip, sizeof(lv)))
+ return -EFAULT;
+
+ mutex_lock(>config_mutex);
+
+ ret = linereq_set_values_unlocked(lr, );
+
+ mutex_unlock(>config_mutex);
+
+ return ret;
+}
+
static long linereq_set_config_unlocked(struct linereq *lr,
struct gpio_v2_line_config *lc)
{
@@ -892,6 +951,8 @@ static long linereq_ioctl(struct file *file, unsigned int
cmd,
if (cmd == GPIO_V2_LINE_GET_VALUES_IOCTL)
return linereq_get_values(lr, ip);
+ else if (cmd == GPIO_V2_LINE_SET_VALUES_IOCTL)
+ return linereq_set_values(lr, ip);
else if (cmd == GPIO_V2_LINE_SET_CONFIG_IOCTL)
return linereq_set_config(lr, ip);
--
2.28.0
[PATCH v10 07/20] gpiolib: cdev: support GPIO_V2_GET_LINE_IOCTL and GPIO_V2_LINE_GET_VALUES_IOCTL
Add support for requesting lines using the GPIO_V2_GET_LINE_IOCTL, and
returning their current values using GPIO_V2_LINE_GET_VALUES_IOCTL.
The struct linereq implementation is based on the v1 struct linehandle
implementation.
Signed-off-by: Kent Gibson
---
The linereq_ioctl() is a simple wrapper around linereq_get_values() here,
but will be extended with other ioctls in subsequent patches.
Similarly, the struct line only contains the desc here, but will receive
the edge detector and debouncer fields in subsequent patches.
Changes for v10:
- as per cover letter
Changes for v8:
- fix BUILD_BUG_ON conditions and relocate them before the return in
gpiolib_cdev_register()
Changes for v7:
- add check on kmalloc_array return value
Changes for v5:
- as per cover letter
Changes for v4:
- fix handling of mask in line_get_values
drivers/gpio/gpiolib-cdev.c | 424
1 file changed, 424 insertions(+)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 86679397d09c..a5f2256cdf8c 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -1,7 +1,9 @@
// SPDX-License-Identifier: GPL-2.0
#include
+#include
#include
+#include
#include
#include
#include
@@ -24,6 +26,25 @@
#include "gpiolib.h"
#include "gpiolib-cdev.h"
+/*
+ * Array sizes must ensure 64-bit alignment and not create holes in the
+ * struct packing.
+ */
+static_assert(IS_ALIGNED(GPIO_V2_LINES_MAX, 2));
+static_assert(IS_ALIGNED(GPIO_MAX_NAME_SIZE, 8));
+
+/*
+ * Check that uAPI structs are 64-bit aligned for 32/64-bit compatibility
+ */
+static_assert(IS_ALIGNED(sizeof(struct gpio_v2_line_attribute), 8));
+static_assert(IS_ALIGNED(sizeof(struct gpio_v2_line_config_attribute), 8));
+static_assert(IS_ALIGNED(sizeof(struct gpio_v2_line_config), 8));
+static_assert(IS_ALIGNED(sizeof(struct gpio_v2_line_request), 8));
+static_assert(IS_ALIGNED(sizeof(struct gpio_v2_line_info), 8));
+static_assert(IS_ALIGNED(sizeof(struct gpio_v2_line_info_changed), 8));
+static_assert(IS_ALIGNED(sizeof(struct gpio_v2_line_event), 8));
+static_assert(IS_ALIGNED(sizeof(struct gpio_v2_line_values), 8));
+
/* Character device interface to GPIO.
*
* The GPIO character device, /dev/gpiochipN, provides userspace an
@@ -34,6 +55,7 @@
* GPIO line handle management
*/
+#ifdef CONFIG_GPIO_CDEV_V1
/**
* struct linehandle_state - contains the state of a userspace handle
* @gdev: the GPIO device the handle pertains to
@@ -376,6 +398,402 @@ static int linehandle_create(struct gpio_device *gdev,
void __user *ip)
linehandle_free(lh);
return ret;
}
+#endif /* CONFIG_GPIO_CDEV_V1 */
+
+/**
+ * struct line - contains the state of a requested line
+ * @desc: the GPIO descriptor for this line.
+ */
+struct line {
+ struct gpio_desc *desc;
+};
+
+/**
+ * struct linereq - contains the state of a userspace line request
+ * @gdev: the GPIO device the line request pertains to
+ * @label: consumer label used to tag GPIO descriptors
+ * @num_lines: the number of lines in the lines array
+ * @lines: the lines held by this line request, with @num_lines elements.
+ */
+struct linereq {
+ struct gpio_device *gdev;
+ const char *label;
+ u32 num_lines;
+ struct line lines[];
+};
+
+#define GPIO_V2_LINE_BIAS_FLAGS \
+ (GPIO_V2_LINE_FLAG_BIAS_PULL_UP | \
+GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN | \
+GPIO_V2_LINE_FLAG_BIAS_DISABLED)
+
+#define GPIO_V2_LINE_DIRECTION_FLAGS \
+ (GPIO_V2_LINE_FLAG_INPUT | \
+GPIO_V2_LINE_FLAG_OUTPUT)
+
+#define GPIO_V2_LINE_DRIVE_FLAGS \
+ (GPIO_V2_LINE_FLAG_OPEN_DRAIN | \
+GPIO_V2_LINE_FLAG_OPEN_SOURCE)
+
+#define GPIO_V2_LINE_VALID_FLAGS \
+ (GPIO_V2_LINE_FLAG_ACTIVE_LOW | \
+GPIO_V2_LINE_DIRECTION_FLAGS | \
+GPIO_V2_LINE_DRIVE_FLAGS | \
+GPIO_V2_LINE_BIAS_FLAGS)
+
+static u64 gpio_v2_line_config_flags(struct gpio_v2_line_config *lc,
+unsigned int line_idx)
+{
+ unsigned int i;
+ u64 mask = BIT_ULL(line_idx);
+
+ for (i = 0; i < lc->num_attrs; i++) {
+ if ((lc->attrs[i].attr.id == GPIO_V2_LINE_ATTR_ID_FLAGS) &&
+ (lc->attrs[i].mask & mask))
+ return lc->attrs[i].attr.flags;
+ }
+ return lc->flags;
+}
+
+static int gpio_v2_line_config_output_value(struct gpio_v2_line_config *lc,
+ unsigned int line_idx)
+{
+ unsigned int i;
+ u64 mask = BIT_ULL(line_idx);
+
+ for (i = 0; i < lc->num_attrs; i++) {
+ if ((lc->attrs[i].attr.id ==
GPIO_V2_LINE_ATTR_ID_OUTPUT_VALUES) &&
+ (lc->attrs[i].mask & mask))
+ return !!(lc->attrs[i].attr.values & mask);
+ }
+ return 0;
+}
+
+static int gpio_v2_line_flags_validate(u64 fl
[PATCH v10 10/20] gpiolib: cdev: support GPIO_V2_LINE_SET_CONFIG_IOCTL
Add support for GPIO_V2_LINE_SET_CONFIG_IOCTL, the uAPI v2
line set config ioctl.
Signed-off-by: Kent Gibson
---
drivers/gpio/gpiolib-cdev.c | 88 +
1 file changed, 88 insertions(+)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 868fcf89478c..608cdbd1d579 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -444,6 +445,8 @@ struct line {
* @seqno: the sequence number for edge events generated on all lines in
* this line request. Note that this is not used when @num_lines is 1, as
* the line_seqno is then the same and is cheaper to calculate.
+ * @config_mutex: mutex for serializing ioctl() calls to ensure consistency
+ * of configuration, particularly multi-step accesses to desc flags.
* @lines: the lines held by this line request, with @num_lines elements.
*/
struct linereq {
@@ -454,6 +457,7 @@ struct linereq {
u32 event_buffer_size;
DECLARE_KFIFO_PTR(events, struct gpio_v2_line_event);
atomic_t seqno;
+ struct mutex config_mutex;
struct line lines[];
};
@@ -574,6 +578,8 @@ static void edge_detector_stop(struct line *line)
free_irq(line->irq, line);
line->irq = 0;
}
+
+ line->eflags = 0;
}
static int edge_detector_setup(struct line *line,
@@ -615,6 +621,17 @@ static int edge_detector_setup(struct line *line,
return 0;
}
+static int edge_detector_update(struct line *line, u64 eflags,
+ bool polarity_change)
+{
+ if ((line->eflags == eflags) && !polarity_change)
+ return 0;
+
+ edge_detector_stop(line);
+
+ return edge_detector_setup(line, eflags);
+}
+
static u64 gpio_v2_line_config_flags(struct gpio_v2_line_config *lc,
unsigned int line_idx)
{
@@ -799,6 +816,74 @@ static long linereq_get_values(struct linereq *lr, void
__user *ip)
return 0;
}
+static long linereq_set_config_unlocked(struct linereq *lr,
+ struct gpio_v2_line_config *lc)
+{
+ struct gpio_desc *desc;
+ unsigned int i;
+ u64 flags;
+ bool polarity_change;
+ int ret;
+
+ for (i = 0; i < lr->num_lines; i++) {
+ desc = lr->lines[i].desc;
+ flags = gpio_v2_line_config_flags(lc, i);
+ polarity_change =
+ (!!test_bit(FLAG_ACTIVE_LOW, >flags) !=
+((flags & GPIO_V2_LINE_FLAG_ACTIVE_LOW) != 0));
+
+ gpio_v2_line_config_flags_to_desc_flags(flags, >flags);
+ /*
+* Lines have to be requested explicitly for input
+* or output, else the line will be treated "as is".
+*/
+ if (flags & GPIO_V2_LINE_FLAG_OUTPUT) {
+ int val = gpio_v2_line_config_output_value(lc, i);
+
+ edge_detector_stop(>lines[i]);
+ ret = gpiod_direction_output(desc, val);
+ if (ret)
+ return ret;
+ } else if (flags & GPIO_V2_LINE_FLAG_INPUT) {
+ ret = gpiod_direction_input(desc);
+ if (ret)
+ return ret;
+
+ ret = edge_detector_update(>lines[i],
+ flags & GPIO_V2_LINE_EDGE_FLAGS,
+ polarity_change);
+ if (ret)
+ return ret;
+ }
+
+ blocking_notifier_call_chain(>gdev->notifier,
+GPIO_V2_LINE_CHANGED_CONFIG,
+desc);
+ }
+ return 0;
+}
+
+static long linereq_set_config(struct linereq *lr, void __user *ip)
+{
+ struct gpio_v2_line_config lc;
+ int ret;
+
+ if (copy_from_user(, ip, sizeof(lc)))
+ return -EFAULT;
+
+ ret = gpio_v2_line_config_validate(, lr->num_lines);
+ if (ret)
+ return ret;
+
+ mutex_lock(>config_mutex);
+
+ ret = linereq_set_config_unlocked(lr, );
+
+ mutex_unlock(>config_mutex);
+
+ return ret;
+}
+
static long linereq_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
@@ -807,6 +892,8 @@ static long linereq_ioctl(struct file *file, unsigned int
cmd,
if (cmd == GPIO_V2_LINE_GET_VALUES_IOCTL)
return linereq_get_values(lr, ip);
+ else if (cmd == GPIO_V2_LINE_SET_CONFIG_IOCTL)
+ return linereq_set_config(lr, ip);
return -EINVAL;
}
@@ -968
[PATCH v10 08/20] gpiolib: cdev: support GPIO_V2_GET_LINEINFO_IOCTL and GPIO_V2_GET_LINEINFO_WATCH_IOCTL
Add support for GPIO_V2_GET_LINEINFO_IOCTL and
GPIO_V2_GET_LINEINFO_WATCH_IOCTL.
The core of this change is the event kfifo switching to contain
struct gpioline_info_changed_v2, instead of v1 as v2 is richer.
The two uAPI versions are mostly independent - other than where they both
provide line info changes via reads on the chip fd. As the info change
structs differ between v1 and v2, the infowatch implementation tracks which
version of the infowatch ioctl, either GPIO_GET_LINEINFO_WATCH_IOCTL or
GPIO_V2_GET_LINEINFO_WATCH_IOCTL, initiates the initial watch and returns
the corresponding info change struct to the read. The version supported
on that fd locks to that version on the first watch request, so subsequent
watches from that process must use the same uAPI version.
Signed-off-by: Kent Gibson
---
Changes for v10:
- as per cover letter
Changes for v5:
- as per cover letter
Changes for v4:
- replace strncpy with memcpy in gpio_v2_line_info_to_v1
drivers/gpio/gpiolib-cdev.c | 196 ++--
1 file changed, 168 insertions(+), 28 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index a5f2256cdf8c..786e667cdc85 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -181,7 +181,8 @@ static long linehandle_set_config(struct linehandle_state
*lh,
}
blocking_notifier_call_chain(>gdev->notifier,
-GPIOLINE_CHANGED_CONFIG, desc);
+GPIO_V2_LINE_CHANGED_CONFIG,
+desc);
}
return 0;
}
@@ -353,7 +354,7 @@ static int linehandle_create(struct gpio_device *gdev, void
__user *ip)
}
blocking_notifier_call_chain(>gdev->notifier,
-GPIOLINE_CHANGED_REQUESTED, desc);
+GPIO_V2_LINE_CHANGED_REQUESTED,
desc);
dev_dbg(>dev, "registered chardev handle for line %d\n",
offset);
@@ -749,7 +750,7 @@ static int linereq_create(struct gpio_device *gdev, void
__user *ip)
}
blocking_notifier_call_chain(>gdev->notifier,
-GPIOLINE_CHANGED_REQUESTED, desc);
+GPIO_V2_LINE_CHANGED_REQUESTED,
desc);
dev_dbg(>dev, "registered chardev handle for line %d\n",
offset);
@@ -1096,7 +1097,7 @@ static int lineevent_create(struct gpio_device *gdev,
void __user *ip)
goto out_free_le;
blocking_notifier_call_chain(>gdev->notifier,
-GPIOLINE_CHANGED_REQUESTED, desc);
+GPIO_V2_LINE_CHANGED_REQUESTED, desc);
irq = gpiod_to_irq(desc);
if (irq <= 0) {
@@ -1163,17 +1164,58 @@ static int lineevent_create(struct gpio_device *gdev,
void __user *ip)
return ret;
}
+static void gpio_v2_line_info_to_v1(struct gpio_v2_line_info *info_v2,
+ struct gpioline_info *info_v1)
+{
+ u64 flagsv2 = info_v2->flags;
+
+ memcpy(info_v1->name, info_v2->name, sizeof(info_v1->name));
+ memcpy(info_v1->consumer, info_v2->consumer, sizeof(info_v1->consumer));
+ info_v1->line_offset = info_v2->offset;
+ info_v1->flags = 0;
+
+ if (flagsv2 & GPIO_V2_LINE_FLAG_USED)
+ info_v1->flags |= GPIOLINE_FLAG_KERNEL;
+
+ if (flagsv2 & GPIO_V2_LINE_FLAG_OUTPUT)
+ info_v1->flags |= GPIOLINE_FLAG_IS_OUT;
+
+ if (flagsv2 & GPIO_V2_LINE_FLAG_ACTIVE_LOW)
+ info_v1->flags |= GPIOLINE_FLAG_ACTIVE_LOW;
+
+ if (flagsv2 & GPIO_V2_LINE_FLAG_OPEN_DRAIN)
+ info_v1->flags |= GPIOLINE_FLAG_OPEN_DRAIN;
+ if (flagsv2 & GPIO_V2_LINE_FLAG_OPEN_SOURCE)
+ info_v1->flags |= GPIOLINE_FLAG_OPEN_SOURCE;
+
+ if (flagsv2 & GPIO_V2_LINE_FLAG_BIAS_PULL_UP)
+ info_v1->flags |= GPIOLINE_FLAG_BIAS_PULL_UP;
+ if (flagsv2 & GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN)
+ info_v1->flags |= GPIOLINE_FLAG_BIAS_PULL_DOWN;
+ if (flagsv2 & GPIO_V2_LINE_FLAG_BIAS_DISABLED)
+ info_v1->flags |= GPIOLINE_FLAG_BIAS_DISABLE;
+}
+
+static void gpio_v2_line_info_changed_to_v1(
+ struct gpio_v2_line_info_changed *lic_v2,
+ struct gpioline_info_changed *lic_v1)
+{
+ gpio_v2_line_info_to_v1(_v2->info, _v1->info);
+ lic_v1->timestamp = lic_v2->timestamp_ns;
+ lic_v1->event_type = lic_v2->event_type;
+}
+
#endif /* CONFIG_GPIO_CDEV_V1 */
static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
-
[PATCH v10 09/20] gpiolib: cdev: support edge detection for uAPI v2
Add support for edge detection to lines requested using
GPIO_V2_GET_LINE_IOCTL.
The edge_detector implementation is based on the v1 lineevent
implementation. Unlike the v1 implementation, an overflow of the event
buffer results in discarding older events, rather than the most
recent, so the final event in a burst will correspond to the current
state of the line.
Signed-off-by: Kent Gibson
---
The linereq_put_event() helper is only used once here, but is re-used in
subsequent patches, and so is pre-emptively split out.
edge_detector_stop() is extended in subsequent patches, and is structured
to suit those additions.
drivers/gpio/gpiolib-cdev.c | 277
drivers/gpio/gpiolib.c | 2 +
drivers/gpio/gpiolib.h | 2 +
3 files changed, 281 insertions(+)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 786e667cdc85..868fcf89478c 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -404,9 +404,33 @@ static int linehandle_create(struct gpio_device *gdev,
void __user *ip)
/**
* struct line - contains the state of a requested line
* @desc: the GPIO descriptor for this line.
+ * @req: the corresponding line request
+ * @irq: the interrupt triggered in response to events on this GPIO
+ * @eflags: the edge flags, GPIO_V2_LINE_FLAG_EDGE_RISING and/or
+ * GPIO_V2_LINE_FLAG_EDGE_FALLING, indicating the edge detection applied
+ * @timestamp_ns: cache for the timestamp storing it between hardirq and
+ * IRQ thread, used to bring the timestamp close to the actual event
+ * @req_seqno: the seqno for the current edge event in the sequence of
+ * events for the corresponding line request. This is drawn from the @req.
+ * @line_seqno: the seqno for the current edge event in the sequence of
+ * events for this line.
*/
struct line {
struct gpio_desc *desc;
+ /*
+* -- edge detector specific fields --
+*/
+ struct linereq *req;
+ unsigned int irq;
+ u64 eflags;
+ /*
+* timestamp_ns and req_seqno are accessed only by
+* edge_irq_handler() and edge_irq_thread(), which are themselves
+* mutually exclusive, so no additional protection is necessary.
+*/
+ u64 timestamp_ns;
+ u32 req_seqno;
+ u32 line_seqno;
};
/**
@@ -414,12 +438,22 @@ struct line {
* @gdev: the GPIO device the line request pertains to
* @label: consumer label used to tag GPIO descriptors
* @num_lines: the number of lines in the lines array
+ * @wait: wait queue that handles blocking reads of events
+ * @event_buffer_size: the number of elements allocated in @events
+ * @events: KFIFO for the GPIO events
+ * @seqno: the sequence number for edge events generated on all lines in
+ * this line request. Note that this is not used when @num_lines is 1, as
+ * the line_seqno is then the same and is cheaper to calculate.
* @lines: the lines held by this line request, with @num_lines elements.
*/
struct linereq {
struct gpio_device *gdev;
const char *label;
u32 num_lines;
+ wait_queue_head_t wait;
+ u32 event_buffer_size;
+ DECLARE_KFIFO_PTR(events, struct gpio_v2_line_event);
+ atomic_t seqno;
struct line lines[];
};
@@ -436,12 +470,151 @@ struct linereq {
(GPIO_V2_LINE_FLAG_OPEN_DRAIN | \
GPIO_V2_LINE_FLAG_OPEN_SOURCE)
+#define GPIO_V2_LINE_EDGE_FLAGS \
+ (GPIO_V2_LINE_FLAG_EDGE_RISING | \
+GPIO_V2_LINE_FLAG_EDGE_FALLING)
+
#define GPIO_V2_LINE_VALID_FLAGS \
(GPIO_V2_LINE_FLAG_ACTIVE_LOW | \
GPIO_V2_LINE_DIRECTION_FLAGS | \
GPIO_V2_LINE_DRIVE_FLAGS | \
+GPIO_V2_LINE_EDGE_FLAGS | \
GPIO_V2_LINE_BIAS_FLAGS)
+static void linereq_put_event(struct linereq *lr,
+ struct gpio_v2_line_event *le)
+{
+ bool overflow = false;
+
+ spin_lock(>wait.lock);
+ if (kfifo_is_full(>events)) {
+ overflow = true;
+ kfifo_skip(>events);
+ }
+ kfifo_in(>events, le, 1);
+ spin_unlock(>wait.lock);
+ if (!overflow)
+ wake_up_poll(>wait, EPOLLIN);
+ else
+ pr_debug_ratelimited("event FIFO is full - event dropped\n");
+}
+
+static irqreturn_t edge_irq_thread(int irq, void *p)
+{
+ struct line *line = p;
+ struct linereq *lr = line->req;
+ struct gpio_v2_line_event le;
+
+ /* Do not leak kernel stack to userspace */
+ memset(, 0, sizeof(le));
+
+ if (line->timestamp_ns) {
+ le.timestamp_ns = line->timestamp_ns;
+ } else {
+ /*
+* We may be running from a nested threaded interrupt in
+* which case we didn't get the timestamp from
+* edge_irq_handler().
+*/
+ le.timestamp_ns = ktime_get_ns();
+ if (lr->num_
[PATCH v10 06/20] gpiolib: add build option for CDEV v1 ABI
Add a build option to allow the removal of the CDEV v1 ABI. Suggested-by: Bartosz Golaszewski Signed-off-by: Kent Gibson Reviewed-by: Andy Shevchenko --- This patch is before the v2 implementation, and is non-functional until that patch, as some parts of that patch would be written slightly differently if removing v1 was not considered. Adding this patch after that would necessitate revisiting the v2 changes, so this ordering results in two simpler patches. drivers/gpio/Kconfig | 12 1 file changed, 12 insertions(+) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index e4debd66d71f..d8d086635929 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -82,6 +82,18 @@ config GPIO_CDEV If unsure, say Y. +config GPIO_CDEV_V1 + bool "Support GPIO ABI Version 1" + default y + depends on GPIO_CDEV + help + Say Y here to support version 1 of the GPIO CDEV ABI. + + This ABI version is deprecated. + Please use the latest ABI for new developments. + + If unsure, say Y. + config GPIO_GENERIC depends on HAS_IOMEM # Only for IOMEM drivers tristate -- 2.28.0
[PATCH v10 00/20] gpio: cdev: add uAPI v2
This patchset defines and implements a new version of the GPIO CDEV uAPI to address existing 32/64-bit alignment issues, add support for debounce, event sequence numbers, and allow for requested lines with different configurations. It provides some future proofing by adding optional configuration fields and padding reserved for future use. The series can be partitioned into three blocks; the first two patches are minor fixes that impact later patches, the next eleven contain the v2 uAPI definition and implementation, and the final seven port the GPIO tools to the v2 uAPI and extend them to use new uAPI features. The more complicated patches include their own commentary where appropriate. Cheers, Kent. Changes for v10: - improve comment readability - replacing '&' with 'and' (patch 07 and 09) - add comment regarding label initialization (patch 07) - don't wrap memcpy line (patch 08) - simplify lineinfo_ensure_watch_version() (patch 08) - reorder if/else in edge_irq_thread() (patch 09) - return ENXIO instead of ENODEV if IRQ request fails (patch 09 and 12) - convert test_bit() result to bool (patch 10) - debounced_value() now returns bool (patch 12) - add missing function () to comments (patch 12) - drop redundant '!= 0' (patch 12) - drop "and will be removed in the future" from comment (patch 13) - make use of _BITULL() (patch 17) Changes for v9: - references to function names should include braces (patch 02) - add time scale suffixes to timestamp (_ns) and debounce period (_us) variables (patch 04 and later) - address multiple review comments (patch 04) - drop mention of future API removal (patch 06) - use static_assert() rather than BUILD_BUG_ON() (patch 07) - change event buffer overflow behaviour to discard old events rather than recent events (patch 09) - add spaces around '*' in '*16' (patch 09) - reword comments regarding field access and locking (patch 09 and 12) Changes for v8: - fix BUILD_BUG_ON conditions and relocate them before the return in gpiolib_cdev_register() (patch 07) Changes for v7: - use _BITULL for ULL flag definitions (patch 04) - add check on kmalloc_array return value in linereq_get_values() (patch 07) and linereq_set_values_unlocked() (patch 11) - restore v1 functions used by gpio-mockup selftests (patch 17) Changes for v6: - flags variable in linereq_create() should be u64 not unsigned long (patch 07) - remove restrictions on configuration changes - any change from one valid state to another valid state is allowed. (patches 09, 10, 12) Changes for v5: All changes for v5 fix issues with the gpiolib-cdev.c implementation, in patches 07-12. The uAPI is unchanged from v4, as is the port of the tools. - use IS_ALIGNED in BUILD_BUG_ON checks (patch 07) - relocate BUILD_BUG_ON checks to gpiolib_cdev_register (patch 07) - s/requies/requires/ (patch 07) - use unsigned int for variables that are never negative - change lineinfo_get() parameter from cmd to bool watch (patch 08) - flagsv2 in gpio_v2_line_info_to_v1() should be u64, not int (patch 08) - change "_locked" suffixed function names to "_unlocked" (patch 10 and 11) - be less eager breaking long lines - move commentary into checkin comment where appropriate - particularly patch 12 - restructure the request/line split - rename struct line to struct linereq, and struct edge_detector to struct line, and relocate the desc field from linereq to line. The linereq name was selected over line_request as function names such as linereq_set_values() are more clearly associated with requests than line_request_set_values(), particularly as there is also a struct line. And linereq is as informative as linerequest, so I went with the shortened form. Changes for v4: - bitmap width clarification in gpiod.h (patch 04) - fix info offset initialisation bug (patch 08 and inserting patch 01) - replace strncpy with strscpy to remove compiler warnings (patch 08 and inserting patch 02) - fix mask handling in line_get_values (patch 07) Changes for v3: - disabling the character device from the build requires EXPERT - uAPI revisions (see patch 02) - replace padding_not_zeroed with calls to memchr_inv - don't use bitops on 64-bit flags as that doesn't work on BE-32 - accept first attribute matching a line in gpio_v2_line_config.attrs rather than the last - rework lsgpio port to uAPI v2 as flags reverted to v1 like layout (since patch v2) - swapped patches 17 and 18 to apply debounce to multiple monitored lines Changes for v2: - split out cleanup patches into a separate series. - split implementation patch into a patch for each ioctl or major feature. - split tool port patch into a patch per tool. - rework uAPI to allow requested lines with different configurations. Kent Gibson (20): gpiolib: cdev: gpio_desc_to_lineinfo() should set info offset gpiolib: cdev: replace strncpy() with strscpy() gpi
[PATCH v10 04/20] gpio: uapi: define uAPI v2
Add a new version of the uAPI to address existing 32/64-bit alignment issues, add support for debounce and event sequence numbers, allow requested lines with different configurations, and provide some future proofing by adding padding reserved for future use. The alignment issue relates to the gpioevent_data, which packs to different sizes on 32-bit and 64-bit platforms. That creates problems for 32-bit apps running on 64-bit kernels. uAPI v2 addresses that particular issue, and the problem more generally, by adding pad fields that explicitly pad structs out to 64-bit boundaries, so they will pack to the same size now, and even if some of the reserved padding is used for __u64 fields in the future. The new structs have been analysed with pahole to ensure that they are sized as expected and contain no implicit padding. The lack of future proofing in v1 makes it impossible to, for example, add the debounce feature that is included in v2. The future proofing is addressed by providing configurable attributes in line config and reserved padding in all structs for future features. Specifically, the line request, config, info, info_changed and event structs receive updated versions and new ioctls. As the majority of the structs and ioctls were being replaced, it is opportune to rework some of the other aspects of the uAPI: v1 has three different flags fields, each with their own separate bit definitions. In v2 that is collapsed to one - gpio_v2_line_flag. The handle and event requests are merged into a single request, the line request, as the two requests were mostly the same other than the edge detection provided by event requests. As a byproduct, the v2 uAPI allows for multiple lines producing edge events on the same line handle. This is a new capability as v1 only supports a single line in an event request. As a consequence, there are now only two types of file handle to be concerned with, the chip and the line, and it is clearer which ioctls apply to which type of handle. There is also some minor renaming of fields for consistency compared to their v1 counterparts, e.g. offset rather than lineoffset or line_offset, and consumer rather than consumer_label. Additionally, v1 GPIOHANDLES_MAX becomes GPIO_V2_LINES_MAX in v2 for clarity, and the gpiohandle_data __u8 array becomes a bitmap in gpio_v2_line_values. The v2 uAPI is mostly a reorganisation and extension of v1, so userspace code, particularly libgpiod, should readily port to it. Signed-off-by: Kent Gibson Reviewed-by: Andy Shevchenko --- Changes for v9: - use time scale suffixes on timestamp (_ns) and debounce period (_us) variables - document all enum values - comment wording tweaks - change some field orders to improve readability Changes for v7: - use _BITULL for flag constants Changes for v4: - clarify bitmap width in GPIO_V2_LINES_MAX description Changes for v3: - relocated commentary into commit description - hard limit max requested lines to 64 so bitmaps always fit in a single u64. - prefix all v2 symbols with GPIO_V2 - 64-bit flag values to ULL - use __aligned_u64 to ensure 64-bit fields are 64-bit aligned - support masked get values, as per set values. Changes for v2: - lower case V1 and V2, except in capitalized names - hyphenate 32/64-bit - rename bitmap field to bits - drop PAD_SIZE consts in favour of hard coded numbers - sort includes - change config flags to __u64 - increase padding of gpioline_event - relocate GPIOLINE_CHANGED enum into v2 section (is common with v1) - rework config to collapse direction, drive, bias and edge enums back into flags and add optional attributes that can be associated with a subset of the requested lines. Changes for v1 (since the RFC): - document the constraints on array sizes to maintain 32/64 alignment - add sequence numbers to gpioline_event - use bitmap for values instead of array of __u8 - gpioline_info_v2 contains gpioline_config instead of its composite fields - provide constants for all array sizes, especially padding - renamed "GPIOLINE_FLAG_V2_KERNEL" to "GPIOLINE_FLAG_V2_USED" - renamed "default_values" to "values" - made gpioline_direction zero based - document clock used in gpioline_event timestamp - add event_buffer_size to gpioline_request - rename debounce to debounce_period - rename lines to num_lines include/uapi/linux/gpio.h | 291 +- 1 file changed, 284 insertions(+), 7 deletions(-) diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h index 285cc10355b2..5904f49399de 100644 --- a/include/uapi/linux/gpio.h +++ b/include/uapi/linux/gpio.h @@ -11,11 +11,14 @@ #ifndef _UAPI_GPIO_H_ #define _UAPI_GPIO_H_ +#include #include #include /* * The maximum size of name and label arrays. + * + * Must be a multiple of 8 to ensure 32/64-bit alignment of structs. */ #define GPIO_MAX_NAME_SIZE 32 @@ -32,6 +35,265 @@ st
[PATCH v10 03/20] gpio: uapi: define GPIO_MAX_NAME_SIZE for array sizes
Replace constant array sizes with a macro constant to clarify the source
of array sizes, provide a place to document any constraints on the size,
and to simplify array sizing in userspace if constructing structs
from their composite fields.
Signed-off-by: Kent Gibson
Reviewed-by: Andy Shevchenko
---
include/uapi/linux/gpio.h | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index 9c27cecf406f..285cc10355b2 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -14,6 +14,11 @@
#include
#include
+/*
+ * The maximum size of name and label arrays.
+ */
+#define GPIO_MAX_NAME_SIZE 32
+
/**
* struct gpiochip_info - Information about a certain GPIO chip
* @name: the Linux kernel name of this GPIO chip
@@ -22,8 +27,8 @@
* @lines: number of GPIO lines on this chip
*/
struct gpiochip_info {
- char name[32];
- char label[32];
+ char name[GPIO_MAX_NAME_SIZE];
+ char label[GPIO_MAX_NAME_SIZE];
__u32 lines;
};
@@ -52,8 +57,8 @@ struct gpiochip_info {
struct gpioline_info {
__u32 line_offset;
__u32 flags;
- char name[32];
- char consumer[32];
+ char name[GPIO_MAX_NAME_SIZE];
+ char consumer[GPIO_MAX_NAME_SIZE];
};
/* Maximum number of requested handles */
@@ -123,7 +128,7 @@ struct gpiohandle_request {
__u32 lineoffsets[GPIOHANDLES_MAX];
__u32 flags;
__u8 default_values[GPIOHANDLES_MAX];
- char consumer_label[32];
+ char consumer_label[GPIO_MAX_NAME_SIZE];
__u32 lines;
int fd;
};
@@ -182,7 +187,7 @@ struct gpioevent_request {
__u32 lineoffset;
__u32 handleflags;
__u32 eventflags;
- char consumer_label[32];
+ char consumer_label[GPIO_MAX_NAME_SIZE];
int fd;
};
--
2.28.0
[PATCH v10 05/20] gpiolib: make cdev a build option
Make the gpiolib-cdev module a build option. This allows the CDEV
interface to be removed from the kernel to reduce kernel size in
applications where is it not required, and provides the parent for
other CDEV interface specific build options to follow.
Suggested-by: Bartosz Golaszewski
Signed-off-by: Kent Gibson
Reviewed-by: Andy Shevchenko
---
drivers/gpio/Kconfig| 17 +++--
drivers/gpio/Makefile | 2 +-
drivers/gpio/gpiolib-cdev.h | 15 +++
3 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 5cfdaf3b004d..e4debd66d71f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -66,8 +66,21 @@ config GPIO_SYSFS
This ABI is deprecated. If you want to use GPIO from userspace,
use the character device /dev/gpiochipN with the appropriate
- ioctl() operations instead. The character device is always
- available.
+ ioctl() operations instead.
+
+config GPIO_CDEV
+ bool
+ prompt "Character device (/dev/gpiochipN) support" if EXPERT
+ default y
+ help
+ Say Y here to add the character device /dev/gpiochipN interface
+ for GPIOs. The character device allows userspace to control GPIOs
+ using ioctl() operations.
+
+ Only say N if you are sure that the GPIO character device is not
+ required.
+
+ If unsure, say Y.
config GPIO_GENERIC
depends on HAS_IOMEM # Only for IOMEM drivers
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 639275eb4e4d..6c3791a55a7b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -6,8 +6,8 @@ ccflags-$(CONFIG_DEBUG_GPIO)+= -DDEBUG
obj-$(CONFIG_GPIOLIB) += gpiolib.o
obj-$(CONFIG_GPIOLIB) += gpiolib-devres.o
obj-$(CONFIG_GPIOLIB) += gpiolib-legacy.o
-obj-$(CONFIG_GPIOLIB) += gpiolib-cdev.o
obj-$(CONFIG_OF_GPIO) += gpiolib-of.o
+obj-$(CONFIG_GPIO_CDEV)+= gpiolib-cdev.o
obj-$(CONFIG_GPIO_SYSFS) += gpiolib-sysfs.o
obj-$(CONFIG_GPIO_ACPI)+= gpiolib-acpi.o
diff --git a/drivers/gpio/gpiolib-cdev.h b/drivers/gpio/gpiolib-cdev.h
index 973578e7ad10..19a4e3d57120 100644
--- a/drivers/gpio/gpiolib-cdev.h
+++ b/drivers/gpio/gpiolib-cdev.h
@@ -5,7 +5,22 @@
#include
+#ifdef CONFIG_GPIO_CDEV
+
int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt);
void gpiolib_cdev_unregister(struct gpio_device *gdev);
+#else
+
+static inline int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
+{
+ return 0;
+}
+
+static inline void gpiolib_cdev_unregister(struct gpio_device *gdev)
+{
+}
+
+#endif /* CONFIG_GPIO_CDEV */
+
#endif /* GPIOLIB_CDEV_H */
--
2.28.0
[PATCH v10 02/20] gpiolib: cdev: replace strncpy() with strscpy()
Replace usage of strncpy() with strscpy() to remove -Wstringop-truncation
warnings.
The structures being populated are zeroed, to prevent stack leakage as
they are returned to userspace, so strscpy() performs the equivalent
function without the warnings.
Reported-by: kernel test robot
Signed-off-by: Kent Gibson
Reviewed-by: Andy Shevchenko
---
The memset in gpio_desc_to_lineinfo(), in conjunction with the strscpy,
is necessary as strncpy zero pads the remainder of the destination.
It also guarantees that the info cannot leak kernel stack to userspace.
This is useful here, but is even more important for the v2 info, that
this function is changed to generate in a subsequent patch, as that
struct contains padding and attribute arrays that also need to be
initialised.
drivers/gpio/gpiolib-cdev.c | 23 +++
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 81ce2020f17b..86679397d09c 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -752,6 +752,7 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
bool ok_for_pinctrl;
unsigned long flags;
+ memset(info, 0, sizeof(*info));
info->line_offset = gpio_chip_hwgpio(desc);
/*
@@ -766,19 +767,11 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
spin_lock_irqsave(_lock, flags);
- if (desc->name) {
- strncpy(info->name, desc->name, sizeof(info->name));
- info->name[sizeof(info->name) - 1] = '\0';
- } else {
- info->name[0] = '\0';
- }
+ if (desc->name)
+ strscpy(info->name, desc->name, sizeof(info->name));
- if (desc->label) {
- strncpy(info->consumer, desc->label, sizeof(info->consumer));
- info->consumer[sizeof(info->consumer) - 1] = '\0';
- } else {
- info->consumer[0] = '\0';
- }
+ if (desc->label)
+ strscpy(info->consumer, desc->label, sizeof(info->consumer));
/*
* Userspace only need to know that the kernel is using this GPIO so
@@ -842,12 +835,10 @@ static long gpio_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
memset(, 0, sizeof(chipinfo));
- strncpy(chipinfo.name, dev_name(>dev),
+ strscpy(chipinfo.name, dev_name(>dev),
sizeof(chipinfo.name));
- chipinfo.name[sizeof(chipinfo.name)-1] = '\0';
- strncpy(chipinfo.label, gdev->label,
+ strscpy(chipinfo.label, gdev->label,
sizeof(chipinfo.label));
- chipinfo.label[sizeof(chipinfo.label)-1] = '\0';
chipinfo.lines = gdev->ngpio;
if (copy_to_user(ip, , sizeof(chipinfo)))
return -EFAULT;
--
2.28.0
[PATCH v10 01/20] gpiolib: cdev: gpio_desc_to_lineinfo() should set info offset
Set the value of the line info offset in gpio_desc_to_lineinfo(), rather than relying on it being passed in the info. This makes the function behave as you would expect from the name - it generates the line info corresponding to a given GPIO desc. Signed-off-by: Kent Gibson Reviewed-by: Andy Shevchenko --- There are some instances where this results in the offset being set when it is already set in the info, but this is clearer especially considering that, as part of the replacement of strncpy with strscpy and to ensure kernel stack cannot be leaked to userspace, the info is initially zeroed in a subsequent patch. drivers/gpio/gpiolib-cdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c index e6c9b78adfc2..81ce2020f17b 100644 --- a/drivers/gpio/gpiolib-cdev.c +++ b/drivers/gpio/gpiolib-cdev.c @@ -752,6 +752,8 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc, bool ok_for_pinctrl; unsigned long flags; + info->line_offset = gpio_chip_hwgpio(desc); + /* * This function takes a mutex so we must check this before taking * the spinlock. @@ -933,7 +935,6 @@ static int lineinfo_changed_notify(struct notifier_block *nb, return NOTIFY_DONE; memset(, 0, sizeof(chg)); - chg.info.line_offset = gpio_chip_hwgpio(desc); chg.event_type = action; chg.timestamp = ktime_get_ns(); gpio_desc_to_lineinfo(desc, ); -- 2.28.0
Re: [PATCH v9 07/20] gpiolib: cdev: support GPIO_V2_GET_LINE_IOCTL and GPIO_V2_LINE_GET_VALUES_IOCTL
On Sun, Sep 27, 2020 at 12:00:04PM +0300, Andy Shevchenko wrote:
> On Sat, Sep 26, 2020 at 12:16 PM Kent Gibson wrote:
> > On Fri, Sep 25, 2020 at 01:06:02PM +0300, Andy Shevchenko wrote:
>
> ...
>
> > Hmmm, there is more to it than I thought - gpiod_request_commit(),
> > which this code eventually calls, interprets a null label (not an
> > empty string) as indicating that the user has not set the label, in
> > which case it will set the desc label to "?". So userspace cannot
> > force the desc label to be empty.
> >
> > We need to keep that label as null in that case to maintain that
> > behaviour.
> >
> > I will add a comment there though.
> >
> > Hmmm, having said that, does this form work for you:
> >
> > if (ulr.consumer[0] != '\0') {
> > /* label is only initialized if consumer is set */
> > lr->label = kstrndup(ulr.consumer, sizeof(ulr.consumer) -
> > 1, GFP_KERNEL);
> > ...
> >
> > It actually compiles slightly larger, I guess due to the extra parameter
> > in the kstrndup() call, but may be slightly more readable??
>
> I really don't want to delay this series, choose what you think is
> better and we may amend it later.
>
OK, as this code is common with v1 I'll leave it as is - we can always
change it for all cases in a later patch.
I think that is everything outstanding for v9, so should have a v10 out
shortly.
Cheers,
Kent.
Re: [PATCH v9 07/20] gpiolib: cdev: support GPIO_V2_GET_LINE_IOCTL and GPIO_V2_LINE_GET_VALUES_IOCTL
On Fri, Sep 25, 2020 at 01:06:02PM +0300, Andy Shevchenko wrote:
> On Thu, Sep 24, 2020 at 11:09 AM Kent Gibson wrote:
> > On Wed, Sep 23, 2020 at 02:11:54PM +0300, Andy Shevchenko wrote:
> > > On Tue, Sep 22, 2020 at 5:35 AM Kent Gibson wrote:
>
...
> > > > + /* Make sure this is terminated */
> > > > + ulr.consumer[sizeof(ulr.consumer)-1] = '\0';
> > > > + if (strlen(ulr.consumer)) {
> > > > + lr->label = kstrdup(ulr.consumer, GFP_KERNEL);
> > > > + if (!lr->label) {
> > > > + ret = -ENOMEM;
> > > > + goto out_free_linereq;
> > > > + }
> > > > + }
> > >
> > > Still don't get why we can\t use kstrndup() here...
> > >
> >
> > I know ;-).
> >
> > Another one directly from v1, and the behaviour there is to leave
> > lr->label nulled if consumer is empty.
> > It just avoids a pointless malloc for the null terminator.
>
> Again, similar as for bitmap API usage, if it makes code cleaner and
> increases readability, I will go for it.
> Also don't forget the army of janitors that won't understand the case
> and simply convert everything that can be converted.
>
Hmmm, there is more to it than I thought - gpiod_request_commit(),
which this code eventually calls, interprets a null label (not an
empty string) as indicating that the user has not set the label, in
which case it will set the desc label to "?". So userspace cannot
force the desc label to be empty.
We need to keep that label as null in that case to maintain that
behaviour.
I will add a comment there though.
Hmmm, having said that, does this form work for you:
if (ulr.consumer[0] != '\0') {
/* label is only initialized if consumer is set */
lr->label = kstrndup(ulr.consumer, sizeof(ulr.consumer) - 1,
GFP_KERNEL);
...
It actually compiles slightly larger, I guess due to the extra parameter
in the kstrndup() call, but may be slightly more readable??
Cheers,
Kent.
Re: [PATCH v9 09/20] gpiolib: cdev: support edge detection for uAPI v2
On Fri, Sep 25, 2020 at 12:35:49PM +0300, Andy Shevchenko wrote:
> On Thu, Sep 24, 2020 at 6:07 AM Kent Gibson wrote:
> > On Wed, Sep 23, 2020 at 06:47:28PM +0300, Andy Shevchenko wrote:
> > > On Tue, Sep 22, 2020 at 5:35 AM Kent Gibson wrote:
>
> ...
>
> > Also, this code is drawn from lineevent_irq_thread(), which is ordered
> > this way.
>
> Negative conditionals are slightly harder to read.
>
> ...
>
> > > > + if (!line->timestamp_ns) {
> > > > + le.timestamp_ns = ktime_get_ns();
> > > > + if (lr->num_lines != 1)
> > > > + line->req_seqno = atomic_inc_return(>seqno);
> > > > + } else {
> > > > + le.timestamp_ns = line->timestamp_ns;
> > > > > + }
> > >
> > > Ditto.
> >
> > Firstly, drawn from lineevent_irq_thread() which is structured this way.
> >
> > In this case the comment relates to the condition being true, so
> > re-ordering the if/else would be confusing - unless the comment were
> > moved into the corresponding body??
>
> Yes.
>
Does that mean I should re-order and move the comment into the body?
That would work for me - the normal case is line->timestamp_ns being
set.
> ...
>
> > > > + irq = gpiod_to_irq(line->desc);
> > > > + if (irq <= 0)
> > > > + return -ENODEV;
> > >
> > > So, you mean this is part of ABI. Can we return more appropriate code,
> > > because getting no IRQ doesn't mean we don't have a device.
> > > Also does 0 case have the same meaning?
> >
> > Firstly, this code is drawn from lineevent_create(), so any changes
> > here should be considered for there as well - though this may
> > constitute an ABI change??
>
> For v1 probably, for v2 we are free to fix this.
>
> > I agree ENODEV doesn't seem right here. Are you ok with ENXIO?
>
> Yes.
>
Will do. And in the debounce patch as well.
> > From gpiod_to_irq():
> >
> > /* Zero means NO_IRQ */
> > if (!retirq)
> > return -ENXIO;
> >
> > so it can't even return a 0 :-| - we're just being cautious.
>
> I would drop = part then.
>
ok, but you'd better not come after me in a subsequent review for not
checking the 0 case!
Cheers,
Kent.
Re: [PATCH v9 11/20] gpiolib: cdev: support GPIO_V2_LINE_SET_VALUES_IOCTL
On Fri, Sep 25, 2020 at 12:57:59PM +0300, Andy Shevchenko wrote:
> On Thu, Sep 24, 2020 at 3:46 PM Kent Gibson wrote:
> > On Thu, Sep 24, 2020 at 03:32:48PM +0800, Kent Gibson wrote:
> > > On Wed, Sep 23, 2020 at 07:18:08PM +0300, Andy Shevchenko wrote:
> > > > On Tue, Sep 22, 2020 at 5:36 AM Kent Gibson
> > > > wrote:
> > > > >
> > > > > Add support for the GPIO_V2_LINE_SET_VALUES_IOCTL.
> > > >
> > > > > +static long linereq_set_values_unlocked(struct linereq *lr,
> > > > > + struct gpio_v2_line_values
> > > > > *lv)
> > > > > +{
> > > > > + DECLARE_BITMAP(vals, GPIO_V2_LINES_MAX);
> > > > > + struct gpio_desc **descs;
> > > > > + unsigned int i, didx, num_set;
> > > > > + int ret;
> > > > > +
> > > > > + bitmap_zero(vals, GPIO_V2_LINES_MAX);
> > > > > + for (num_set = 0, i = 0; i < lr->num_lines; i++) {
> > > > > + if (lv->mask & BIT_ULL(i)) {
> > > >
> > > > Similar idea
> > > >
> > > > DECLARE_BITMAP(mask, 64) = BITMAP_FROM_U64(lv->mask);
> > > >
> > > > num_set = bitmap_weight();
> > > >
> > >
> > > I had played with this option, but bitmap_weight() counts all
> > > the bits set in the mask - which considers bits >= lr->num_lines.
> > > So you would need to mask lv->mask before converting it to a bitmap.
> > > (I'm ok with ignoring those bits in case userspace wants to be lazy and
> > > use an all 1s mask.)
> > >
> > > But since we're looping over the bitmap anyway we may as well just
> > > count as we go.
> > >
> > > > for_each_set_bit(i, mask, lr->num_lines)
> > > >
> > >
> > > Yeah, that should work. I vaguely recall trying this and finding it
> > > generated larger object code, but I'll give it another try and if it
> > > works out then include it in v10.
> > >
> >
> > Tried it again and, while it works, it does increase the size of
> > gpiolib-cdev.o as follows:
> >
> > u64 -> bitmap
> > x86_64 28360 28616
> > i386 22056 22100
> > aarch64 37392 37600
> > mips32 28008 28016
>
> Yes, that's pity... See below.
>
> > So for 64-bit platforms changing to bitmap generates larger code,
> > probably as we are forcing them to use 32-bit array semantics where
> > before they could use the native u64. For 32-bit there is a much
> > smaller difference as they were already using 32-bit array semantics
> > to realise the u64.
> >
> > Those are for some of my test builds, so obviously YMMV.
> >
> > It is also only for changing linereq_get_values(), which has three
> > instances of the loop. linereq_set_values_unlocked() has another two,
> > so you could expect another increase of ~2/3 of that seen here if we
> > change that as well.
> >
> > The sizeable increase in x86_64 was what made me revert this last time,
> > and I'm still satisfied with that choice. Are you still eager to switch
> > to for_each_set_bit()?
>
> I already asked once about short cut for for_each_set_bit in case of
> constant nbits parameter when it's <= BITS_PER_LONG, but here it seems
> we have variadic amount of lines, dunno if compiler can prove that
> it's smaller than long. In any case my point is that code readability
> has a preference vs. memory footprint (except hot paths) and if we are
> going to fix this it should be done in general. That said, if
> maintainers are okay with that I would prefer bitmap API over
> open-coded pieces.
>
Agreed - if the bitmap ops made better use of the architecure then I'd
change to bitmap without question - it is more readable.
Bart and Linus - do you have any preference?
> Also note, that it will be easier to extend in the future if needed
> (if we want to have more than BITS_PER_LONG [64] lines to handle).
>
Yeah, I think we can rule that one out - while I had initially written
the uAPI with the option of > 64 lines we decided on the hard limit to
keep things simple. And any user that needs to request more than 64
lines in one request shouldn't be using the GPIO uAPI.
Cheers,
Kent.
