From ac2891d920f1988b94a85220a1f05262c31ee4d7 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 11 Oct 2017 11:25:17 +0200 Subject: [PATCH] event: rework gpiod_line_event_wait_bulk() When monitoring more than one line, it's possible for two or more events to be queued at the same time in the kernel, so the internal call to ppoll() will return a value greater than 1. If we always only read one event at most, we end up calling ppoll() needlessly, as we already know more events are pending. Allow gpiod_line_event_wait_bulk() to pass a list of all lines on which events occurred via a line bulk object. Signed-off-by: Bartosz Golaszewski --- include/gpiod.h | 12 ++++++------ src/lib/core.c | 34 ++++++++++++++++++++++------------ tests/tests-event.c | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 18 deletions(-) diff --git a/include/gpiod.h b/include/gpiod.h index 438744f..702fb87 100644 --- a/include/gpiod.h +++ b/include/gpiod.h @@ -980,17 +980,17 @@ int gpiod_line_event_wait(struct gpiod_line *line, const struct timespec *timeout) GPIOD_API; /** - * @brief Wait for the first event on a set of lines. + * @brief Wait for events on a set of lines. * @param bulk Set of GPIO lines to monitor. * @param timeout Wait time limit. - * @param line The handle of the line on which an event occurs is stored - * in this variable. Can be NULL. - * @return 0 if wait timed out, -1 if an error occurred, 1 if an event - * occurred. + * @param event_bulk Bulk object in which to store the line handles on which + * events occurred. Can be NULL. + * @return 0 if wait timed out, -1 if an error occurred, 1 if at least one + * event occurred. */ int gpiod_line_event_wait_bulk(struct gpiod_line_bulk *bulk, const struct timespec *timeout, - struct gpiod_line **line) GPIOD_API; + struct gpiod_line_bulk *event_bulk) GPIOD_API; /** * @brief Read the last event from the GPIO line. diff --git a/src/lib/core.c b/src/lib/core.c index 0b2c823..b0b42cd 100644 --- a/src/lib/core.c +++ b/src/lib/core.c @@ -993,31 +993,41 @@ int gpiod_line_event_wait(struct gpiod_line *line, int gpiod_line_event_wait_bulk(struct gpiod_line_bulk *bulk, const struct timespec *timeout, - struct gpiod_line **line) + struct gpiod_line_bulk *event_bulk) { struct pollfd fds[GPIOD_LINE_BULK_MAX_LINES]; - struct gpiod_line *linetmp, **lineptr; - unsigned int i; - int status; + struct gpiod_line *line, **lineptr; + unsigned int i, num_lines; + int rv; memset(fds, 0, sizeof(fds)); + num_lines = gpiod_line_bulk_num_lines(bulk); i = 0; - gpiod_line_bulk_foreach_line(bulk, linetmp, lineptr) { - fds[i].fd = line_get_event_fd(linetmp); + gpiod_line_bulk_foreach_line(bulk, line, lineptr) { + fds[i].fd = line_get_event_fd(line); fds[i].events = POLLIN | POLLPRI; i++; } - status = ppoll(fds, gpiod_line_bulk_num_lines(bulk), timeout, NULL); - if (status < 0) + rv = ppoll(fds, num_lines, timeout, NULL); + if (rv < 0) return -1; - else if (status == 0) + else if (rv == 0) return 0; - for (i = 0; !fds[i].revents; i++); - if (line) - *line = gpiod_line_bulk_get_line(bulk, i); + if (event_bulk) { + gpiod_line_bulk_init(event_bulk); + + for (i = 0; i < num_lines; i++) { + if (fds[i].revents) { + line = gpiod_line_bulk_get_line(bulk, i); + gpiod_line_bulk_add(event_bulk, line); + if (!--rv) + break; + } + } + } return 1; } diff --git a/tests/tests-event.c b/tests/tests-event.c index fb14319..423c6d0 100644 --- a/tests/tests-event.c +++ b/tests/tests-event.c @@ -167,3 +167,39 @@ static void event_get_value(void) TEST_DEFINE(event_get_value, "events - mixing events and gpiod_line_get_value()", 0, { 8 }); + +static void event_wait_multiple(void) +{ + TEST_CLEANUP(test_close_chip) struct gpiod_chip *chip = NULL; + struct gpiod_line_bulk bulk, event_bulk; + struct timespec ts = { 1, 0 }; + struct gpiod_line *line; + int rv, i; + + chip = gpiod_chip_open(test_chip_path(0)); + TEST_ASSERT_NOT_NULL(chip); + + gpiod_line_bulk_init(&bulk); + + for (i = 0; i < 8; i++) { + line = gpiod_chip_get_line(chip, i); + TEST_ASSERT_NOT_NULL(line); + + gpiod_line_bulk_add(&bulk, line); + } + + rv = gpiod_line_request_bulk_both_edges_events(&bulk, TEST_CONSUMER); + TEST_ASSERT_RET_OK(rv); + + test_set_event(0, 4, TEST_EVENT_RISING, 100); + + rv = gpiod_line_event_wait_bulk(&bulk, &ts, &event_bulk); + TEST_ASSERT_EQ(rv, 1); + + TEST_ASSERT_EQ(gpiod_line_bulk_num_lines(&event_bulk), 1); + line = gpiod_line_bulk_get_line(&event_bulk, 0); + TEST_ASSERT_EQ(gpiod_line_offset(line), 4); +} +TEST_DEFINE(event_wait_multiple, + "events - wait for events on multiple lines", + 0, { 8 }); -- 2.30.2