[libgpiod v2][PATCH 0/3] libgpiod v2: new API improvements

[libgpiod v2][PATCH 0/3] libgpiod v2: new API improvements

[Bartosz Golaszewski]

These patches add new features and improvements to the core C library API.

Bartosz Golaszewski (3):
  chip: provide gpiod_chip_get_path()
  line-info: provide gpiod_line_info_get_event_clock()
  treewide: unify the line settings defines

 include/gpiod.h      | 118 +++++++++++++++++--------------------------
 lib/chip.c           |  15 +++++-
 lib/line-config.c    |  38 +++++++-------
 lib/line-info.c      |  11 ++++
 tools/gpioget.c      |   4 +-
 tools/gpiomon.c      |   6 +--
 tools/gpioset.c      |   7 ++-
 tools/tools-common.c |   6 +--
 8 files changed, 101 insertions(+), 104 deletions(-)

-- 
2.30.1

[libgpiod v2][PATCH 1/3] chip: provide gpiod_chip_get_path()

[Bartosz Golaszewski]

Provide a chip getter for retrieving the path to the device file used to
get the GPIO chip handle.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 include/gpiod.h |  7 +++++++
 lib/chip.c      | 15 ++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/include/gpiod.h b/include/gpiod.h
index 56dc986..d3c327b 100644
--- a/include/gpiod.h
+++ b/include/gpiod.h
@@ -88,6 +88,13 @@ const char *gpiod_chip_get_name(struct gpiod_chip *chip);
  */
 const char *gpiod_chip_get_label(struct gpiod_chip *chip);
 
+/**
+ * @brief Get the path used to open this GPIO chip.
+ * @param chip GPIO chip object.
+ * @return Path to the file passed as argument to ::gpiod_chip_open.
+ */
+const char *gpiod_chip_get_path(struct gpiod_chip *chip);
+
 /**
  * @brief Get the number of GPIO lines exposed by this chip.
  * @param chip GPIO chip object.
diff --git a/lib/chip.c b/lib/chip.c
index a200ef2..6fcada9 100644
--- a/lib/chip.c
+++ b/lib/chip.c
@@ -18,6 +18,7 @@ struct gpiod_chip {
 	unsigned int num_lines;
 	char name[32];
 	char label[32];
+	char *path;
 };
 
 GPIOD_API struct gpiod_chip *gpiod_chip_open(const char *path)
@@ -40,9 +41,13 @@ GPIOD_API struct gpiod_chip *gpiod_chip_open(const char *path)
 	memset(chip, 0, sizeof(*chip));
 	memset(&info, 0, sizeof(info));
 
+	chip->path = strdup(path);
+	if (!chip->path)
+		goto err_free_chip;
+
 	ret = ioctl(fd, GPIO_GET_CHIPINFO_IOCTL, &info);
 	if (ret < 0)
-		goto err_free_chip;
+		goto err_free_path;
 
 	chip->fd = fd;
 	chip->num_lines = info.lines;
@@ -65,6 +70,8 @@ GPIOD_API struct gpiod_chip *gpiod_chip_open(const char *path)
 
 	return chip;
 
+err_free_path:
+	free(chip->path);
 err_free_chip:
 	free(chip);
 err_close_fd:
@@ -79,6 +86,7 @@ GPIOD_API void gpiod_chip_close(struct gpiod_chip *chip)
 		return;
 
 	close(chip->fd);
+	free(chip->path);
 	free(chip);
 }
 
@@ -92,6 +100,11 @@ GPIOD_API const char *gpiod_chip_get_label(struct gpiod_chip *chip)
 	return chip->label;
 }
 
+GPIOD_API const char *gpiod_chip_get_path(struct gpiod_chip *chip)
+{
+	return chip->path;
+}
+
 GPIOD_API unsigned int gpiod_chip_get_num_lines(struct gpiod_chip *chip)
 {
 	return chip->num_lines;
-- 
2.30.1

[libgpiod v2][PATCH 2/3] line-info: provide gpiod_line_info_get_event_clock()

[Bartosz Golaszewski]

Provide a new getter for the line-info object that becomes the
counterpart for the setter used to configure the event clock type with
line_config. It allows to check which even clock the line uses for
edge detection.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 include/gpiod.h | 18 ++++++++++++++++++
 lib/line-info.c | 11 +++++++++++
 2 files changed, 29 insertions(+)

diff --git a/include/gpiod.h b/include/gpiod.h
index d3c327b..4506571 100644
--- a/include/gpiod.h
+++ b/include/gpiod.h
@@ -245,6 +245,16 @@ enum {
 	/**< Line detects both rising and falling edge events. */
 };
 
+/**
+ * @brief Possible event clock settings.
+ */
+enum {
+	GPIOD_LINE_EVENT_CLOCK_MONOTONIC = 1,
+	/**< Line uses the monotonic clock for edge event timestamps. */
+	GPIOD_LINE_EVENT_CLOCK_REALTIME,
+	/**< Line uses the realtime clock for edge event timestamps. */
+};
+
 /**
  * @brief Free a line info object and release all associated resources.
  * @param info GPIO line info object to free.
@@ -333,6 +343,14 @@ int gpiod_line_info_get_drive(struct gpiod_line_info *info);
  */
 int gpiod_line_info_get_edge_detection(struct gpiod_line_info *info);
 
+/**
+ * @brief Read the current event clock setting used for edge event timestamps.
+ * @param info GPIO line info object.
+ * @return Returns GPIOD_LINE_EVENT_CLOCK_MONOTONIC or
+ *         GPIOD_LINE_EVENT_CLOCK_REALTIME.
+ */
+int gpiod_line_info_get_event_clock(struct gpiod_line_info *info);
+
 /**
  * @brief Check if this line is debounced (either by hardware or by the kernel
  *        software debouncer).
diff --git a/lib/line-info.c b/lib/line-info.c
index aed8bee..0280981 100644
--- a/lib/line-info.c
+++ b/lib/line-info.c
@@ -17,6 +17,7 @@ struct gpiod_line_info {
 	int bias;
 	int drive;
 	int edge;
+	int event_clock;
 	bool debounced;
 	unsigned long debounce_period;
 };
@@ -86,6 +87,11 @@ GPIOD_API int gpiod_line_info_get_edge_detection(struct gpiod_line_info *info)
 	return info->edge;
 }
 
+GPIOD_API int gpiod_line_info_get_event_clock(struct gpiod_line_info *info)
+{
+	return info->event_clock;
+}
+
 GPIOD_API bool gpiod_line_info_is_debounced(struct gpiod_line_info *info)
 {
 	return info->debounced;
@@ -150,6 +156,11 @@ gpiod_line_info_from_kernel(struct gpio_v2_line_info *infobuf)
 	else
 		info->edge = GPIOD_LINE_EDGE_NONE;
 
+	if (infobuf->flags & GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME)
+		info->event_clock = GPIOD_LINE_EVENT_CLOCK_REALTIME;
+	else
+		info->event_clock = GPIOD_LINE_EVENT_CLOCK_MONOTONIC;
+
 	/*
 	 * We assume that the kernel returns correct configuration and that no
 	 * attributes repeat.
-- 
2.30.1

Re: [libgpiod v2][PATCH 2/3] line-info: provide gpiod_line_info_get_event_clock()

[Kent Gibson]

On Fri, Jul 30, 2021 at 04:43:55PM +0200, Bartosz Golaszewski wrote:
> Provide a new getter for the line-info object that becomes the
> counterpart for the setter used to configure the event clock type with
> line_config. It allows to check which even clock the line uses for
> edge detection.
> 

"to check which even" -> "checking which event"

That is my only comment for the whole patch set - eveything else looks
good.

Cheers,
Kent.

Re: [libgpiod v2][PATCH 2/3] line-info: provide gpiod_line_info_get_event_clock()

[Bartosz Golaszewski]

On Sat, Jul 31, 2021 at 4:27 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Fri, Jul 30, 2021 at 04:43:55PM +0200, Bartosz Golaszewski wrote:
> > Provide a new getter for the line-info object that becomes the
> > counterpart for the setter used to configure the event clock type with
> > line_config. It allows to check which even clock the line uses for
> > edge detection.
> >
>
> "to check which even" -> "checking which event"
>
> That is my only comment for the whole patch set - eveything else looks
> good.
>
> Cheers,
> Kent.

This will be squashed with the big v2 commit, thanks!

Bart

[libgpiod v2][PATCH 3/3] treewide: unify the line settings defines

[Bartosz Golaszewski]

The definitions of line settings are mostly duplicated between the
line-info and line-config objects. Unify them by merging any differences
into common enums.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 include/gpiod.h      | 93 ++++++++++----------------------------------
 lib/line-config.c    | 38 +++++++++---------
 tools/gpioget.c      |  4 +-
 tools/gpiomon.c      |  6 +--
 tools/gpioset.c      |  7 ++--
 tools/tools-common.c |  6 +--
 6 files changed, 51 insertions(+), 103 deletions(-)

diff --git a/include/gpiod.h b/include/gpiod.h
index 4506571..85a671f 100644
--- a/include/gpiod.h
+++ b/include/gpiod.h
@@ -185,21 +185,19 @@ gpiod_chip_request_lines(struct gpiod_chip *chip,
 /**
  * @}
  *
- * @defgroup line_info Line info
+ * @defgroup line_settings Line settings
  * @{
  *
- * Definitions and functions for retrieving kernel information about both
- * requested and free lines.
- *
- * Line info object contains an immutable snapshot of the line's state at the
- * time when it was created.
+ * These defines are used both by gpiod_line_info and gpiod_line_config.
  */
 
 /**
  * @brief Direction settings.
  */
 enum {
-	GPIOD_LINE_DIRECTION_INPUT = 1,
+	GPIOD_LINE_DIRECTION_AS_IS = 1,
+	/**< Request the line(s), but don't change current direction. */
+	GPIOD_LINE_DIRECTION_INPUT,
 	/**< Direction is input - we're reading the state of a GPIO line. */
 	GPIOD_LINE_DIRECTION_OUTPUT
 	/**< Direction is output - we're driving the GPIO line. */
@@ -209,7 +207,9 @@ enum {
  * @brief Internal bias settings.
  */
 enum {
-	GPIOD_LINE_BIAS_UNKNOWN = 1,
+	GPIOD_LINE_BIAS_AS_IS = 1,
+	/**< Don't change the bias setting when applying line config. */
+	GPIOD_LINE_BIAS_UNKNOWN,
 	/**< The internal bias state is unknown. */
 	GPIOD_LINE_BIAS_DISABLED,
 	/**< The internal bias is disabled. */
@@ -255,6 +255,19 @@ enum {
 	/**< Line uses the realtime clock for edge event timestamps. */
 };
 
+/**
+ * @}
+ *
+ * @defgroup line_info Line info
+ * @{
+ *
+ * Functions for retrieving kernel information about both requested and free
+ * lines.
+ *
+ * Line info object contains an immutable snapshot of the line's state at the
+ * time when it was created.
+ */
+
 /**
  * @brief Free a line info object and release all associated resources.
  * @param info GPIO line info object to free.
@@ -445,70 +458,6 @@ gpiod_info_event_get_line_info(struct gpiod_info_event *event);
  * a single line offset or for multiple line offsets.
  */
 
-/**
- * @brief Available direction settings.
- */
-enum {
-	GPIOD_LINE_CONFIG_DIRECTION_AS_IS = 1,
-	/**< Request the line(s), but don't change current direction. */
-	GPIOD_LINE_CONFIG_DIRECTION_INPUT,
-	/**< Request the line(s) for reading the GPIO line state. */
-	GPIOD_LINE_CONFIG_DIRECTION_OUTPUT
-	/**< Request the line(s) for setting the GPIO line state. */
-};
-
-/**
- * @brief Available edge event detection settings. Only relevant for input
- *        direction.
- */
-enum {
-	GPIOD_LINE_CONFIG_EDGE_NONE = 1,
-	/**< Don't report edge events. */
-	GPIOD_LINE_CONFIG_EDGE_FALLING,
-	/**< Only watch falling edge events. */
-	GPIOD_LINE_CONFIG_EDGE_RISING,
-	/**< Only watch rising edge events. */
-	GPIOD_LINE_CONFIG_EDGE_BOTH
-	/**< Monitor both types of events. */
-};
-
-/**
- * @brief Available internal bias settings for line requests.
- */
-enum {
-	GPIOD_LINE_CONFIG_BIAS_AS_IS = 1,
-	/**< Don't change the current bias setting. */
-	GPIOD_LINE_CONFIG_BIAS_DISABLED,
-	/**< The internal bias should be disabled (the default). */
-	GPIOD_LINE_CONFIG_BIAS_PULL_UP,
-	/**< The internal pull-up bias is enabled. */
-	GPIOD_LINE_CONFIG_BIAS_PULL_DOWN
-	/**< The internal pull-down bias is enabled. */
-};
-
-/**
- * @brief Available drive settings for line requests. Only relevant for output
- *        direction.
- */
-enum {
-	GPIOD_LINE_CONFIG_DRIVE_PUSH_PULL = 1,
-	/**< Drive setting should be set to push-pull (the default). */
-	GPIOD_LINE_CONFIG_DRIVE_OPEN_DRAIN,
-	/**< Line output should be set to open-drain. */
-	GPIOD_LINE_CONFIG_DRIVE_OPEN_SOURCE
-	/**< Line output should be set to open-source. */
-};
-
-/**
- * @brief Available clock types used for event timestamps.
- */
-enum {
-	GPIOD_LINE_CONFIG_EVENT_CLOCK_MONOTONIC = 1,
-	/**< Use the monotonic clock. */
-	GPIOD_LINE_CONFIG_EVENT_CLOCK_REALTIME
-	/**< Use the realtime clock. */
-};
-
 /**
  * @brief Create a new line config object.
  * @return New line config object or NULL on error.
diff --git a/lib/line-config.c b/lib/line-config.c
index 24dc5f4..6cc4676 100644
--- a/lib/line-config.c
+++ b/lib/line-config.c
@@ -435,13 +435,13 @@ static int gpiod_make_kernel_flags(uint64_t *flags, struct base_config *config)
 	*flags = 0;
 
 	switch (config->direction) {
-	case GPIOD_LINE_CONFIG_DIRECTION_INPUT:
+	case GPIOD_LINE_DIRECTION_INPUT:
 		*flags |= GPIO_V2_LINE_FLAG_INPUT;
 		break;
-	case GPIOD_LINE_CONFIG_DIRECTION_OUTPUT:
+	case GPIOD_LINE_DIRECTION_OUTPUT:
 		*flags |= GPIO_V2_LINE_FLAG_OUTPUT;
 		break;
-	case GPIOD_LINE_CONFIG_DIRECTION_AS_IS:
+	case GPIOD_LINE_DIRECTION_AS_IS:
 	case 0:
 		break;
 	default:
@@ -449,23 +449,23 @@ static int gpiod_make_kernel_flags(uint64_t *flags, struct base_config *config)
 	}
 
 	switch (config->edge) {
-	case GPIOD_LINE_CONFIG_EDGE_FALLING:
+	case GPIOD_LINE_EDGE_FALLING:
 		*flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING |
 			   GPIO_V2_LINE_FLAG_INPUT);
-		*flags &= ~GPIOD_LINE_CONFIG_DIRECTION_OUTPUT;
+		*flags &= ~GPIOD_LINE_DIRECTION_OUTPUT;
 		break;
-	case GPIOD_LINE_CONFIG_EDGE_RISING:
+	case GPIOD_LINE_EDGE_RISING:
 		*flags |= (GPIO_V2_LINE_FLAG_EDGE_RISING |
 			   GPIO_V2_LINE_FLAG_INPUT);
-		*flags &= ~GPIOD_LINE_CONFIG_DIRECTION_OUTPUT;
+		*flags &= ~GPIOD_LINE_DIRECTION_OUTPUT;
 		break;
-	case GPIOD_LINE_CONFIG_EDGE_BOTH:
+	case GPIOD_LINE_EDGE_BOTH:
 		*flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING |
 			   GPIO_V2_LINE_FLAG_EDGE_RISING |
 			   GPIO_V2_LINE_FLAG_INPUT);
-		*flags &= ~GPIOD_LINE_CONFIG_DIRECTION_OUTPUT;
+		*flags &= ~GPIOD_LINE_DIRECTION_OUTPUT;
 		break;
-	case GPIOD_LINE_CONFIG_EDGE_NONE:
+	case GPIOD_LINE_EDGE_NONE:
 	case 0:
 		break;
 	default:
@@ -473,13 +473,13 @@ static int gpiod_make_kernel_flags(uint64_t *flags, struct base_config *config)
 	}
 
 	switch (config->drive) {
-	case GPIOD_LINE_CONFIG_DRIVE_OPEN_DRAIN:
+	case GPIOD_LINE_DRIVE_OPEN_DRAIN:
 		*flags |= GPIO_V2_LINE_FLAG_OPEN_DRAIN;
 		break;
-	case GPIOD_LINE_CONFIG_DRIVE_OPEN_SOURCE:
+	case GPIOD_LINE_DRIVE_OPEN_SOURCE:
 		*flags |= GPIO_V2_LINE_FLAG_OPEN_SOURCE;
 		break;
-	case GPIOD_LINE_CONFIG_DRIVE_PUSH_PULL:
+	case GPIOD_LINE_DRIVE_PUSH_PULL:
 	case 0:
 		break;
 	default:
@@ -487,16 +487,16 @@ static int gpiod_make_kernel_flags(uint64_t *flags, struct base_config *config)
 	}
 
 	switch (config->bias) {
-	case GPIOD_LINE_CONFIG_BIAS_DISABLED:
+	case GPIOD_LINE_BIAS_DISABLED:
 		*flags |= GPIO_V2_LINE_FLAG_BIAS_DISABLED;
 		break;
-	case GPIOD_LINE_CONFIG_BIAS_PULL_UP:
+	case GPIOD_LINE_BIAS_PULL_UP:
 		*flags |= GPIO_V2_LINE_FLAG_BIAS_PULL_UP;
 		break;
-	case GPIOD_LINE_CONFIG_BIAS_PULL_DOWN:
+	case GPIOD_LINE_BIAS_PULL_DOWN:
 		*flags |= GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN;
 		break;
-	case GPIOD_LINE_CONFIG_BIAS_AS_IS:
+	case GPIOD_LINE_BIAS_AS_IS:
 	case 0:
 		break;
 	default:
@@ -507,10 +507,10 @@ static int gpiod_make_kernel_flags(uint64_t *flags, struct base_config *config)
 		*flags |= GPIO_V2_LINE_FLAG_ACTIVE_LOW;
 
 	switch (config->clock) {
-	case GPIOD_LINE_CONFIG_EVENT_CLOCK_REALTIME:
+	case GPIOD_LINE_EVENT_CLOCK_REALTIME:
 		*flags |= GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME;
 		break;
-	case GPIOD_LINE_CONFIG_EVENT_CLOCK_MONOTONIC:
+	case GPIOD_LINE_EVENT_CLOCK_MONOTONIC:
 	case 0:
 		break;
 	default:
diff --git a/tools/gpioget.c b/tools/gpioget.c
index afa2ccb..965af3b 100644
--- a/tools/gpioget.c
+++ b/tools/gpioget.c
@@ -41,7 +41,7 @@ static void print_help(void)
 
 int main(int argc, char **argv)
 {
-	int direction = GPIOD_LINE_CONFIG_DIRECTION_INPUT;
+	int direction = GPIOD_LINE_DIRECTION_INPUT;
 	int optc, opti, bias = 0, ret, *values;
 	struct gpiod_request_config *req_cfg;
 	unsigned int *offsets, i, num_lines;
@@ -67,7 +67,7 @@ int main(int argc, char **argv)
 			active_low = true;
 			break;
 		case 'n':
-			direction = GPIOD_LINE_CONFIG_DIRECTION_AS_IS;
+			direction = GPIOD_LINE_DIRECTION_AS_IS;
 			break;
 		case 'B':
 			bias = parse_bias(optarg);
diff --git a/tools/gpiomon.c b/tools/gpiomon.c
index 66fbb13..740c249 100644
--- a/tools/gpiomon.c
+++ b/tools/gpiomon.c
@@ -225,11 +225,11 @@ int main(int argc, char **argv)
 	argv += optind;
 
 	if (watch_rising && !watch_falling)
-		edge = GPIOD_LINE_CONFIG_EDGE_RISING;
+		edge = GPIOD_LINE_EDGE_RISING;
 	else if (watch_falling && !watch_rising)
-		edge = GPIOD_LINE_CONFIG_EDGE_FALLING;
+		edge = GPIOD_LINE_EDGE_FALLING;
 	else
-		edge = GPIOD_LINE_CONFIG_EDGE_BOTH;
+		edge = GPIOD_LINE_EDGE_BOTH;
 
 	if (argc < 1)
 		die("gpiochip must be specified");
diff --git a/tools/gpioset.c b/tools/gpioset.c
index bf90087..55fcfe9 100644
--- a/tools/gpioset.c
+++ b/tools/gpioset.c
@@ -179,9 +179,9 @@ static const struct mode_mapping *parse_mode(const char *mode)
 static int parse_drive(const char *option)
 {
 	if (strcmp(option, "open-drain") == 0)
-		return GPIOD_LINE_CONFIG_DRIVE_OPEN_DRAIN;
+		return GPIOD_LINE_DRIVE_OPEN_DRAIN;
 	if (strcmp(option, "open-source") == 0)
-		return GPIOD_LINE_CONFIG_DRIVE_OPEN_SOURCE;
+		return GPIOD_LINE_DRIVE_OPEN_SOURCE;
 	if (strcmp(option, "push-pull") != 0)
 		die("invalid drive: %s", option);
 	return 0;
@@ -301,8 +301,7 @@ int main(int argc, char **argv)
 		gpiod_line_config_set_drive(line_cfg, drive);
 	if (active_low)
 		gpiod_line_config_set_active_low(line_cfg);
-	gpiod_line_config_set_direction(line_cfg,
-					GPIOD_LINE_CONFIG_DIRECTION_OUTPUT);
+	gpiod_line_config_set_direction(line_cfg, GPIOD_LINE_DIRECTION_OUTPUT);
 	gpiod_line_config_set_output_values(line_cfg, num_lines,
 					    offsets, values);
 
diff --git a/tools/tools-common.c b/tools/tools-common.c
index 36724d5..f5fd50c 100644
--- a/tools/tools-common.c
+++ b/tools/tools-common.c
@@ -60,11 +60,11 @@ void print_version(void)
 int parse_bias(const char *option)
 {
 	if (strcmp(option, "pull-down") == 0)
-		return GPIOD_LINE_CONFIG_BIAS_PULL_DOWN;
+		return GPIOD_LINE_BIAS_PULL_DOWN;
 	if (strcmp(option, "pull-up") == 0)
-		return GPIOD_LINE_CONFIG_BIAS_PULL_UP;
+		return GPIOD_LINE_BIAS_PULL_UP;
 	if (strcmp(option, "disable") == 0)
-		return GPIOD_LINE_CONFIG_BIAS_DISABLED;
+		return GPIOD_LINE_BIAS_DISABLED;
 	if (strcmp(option, "as-is") != 0)
 		die("invalid bias: %s", option);
 	return 0;
-- 
2.30.1