This is an automated email from Gerrit.
"Michael Heimpold <m...@heimpold.de>" just uploaded a new patch set to Gerrit,
which you can find at https://review.openocd.org/c/openocd/+/8184
-- gerrit
commit acd26894d3bf2ac36a1776947dd3e3c397b957b5
Author: Michael Heimpold <m...@heimpold.de>
Date: Thu Mar 21 07:34:18 2024 +0100
drivers/linuxgpiodv2: introduce new driver for libgpiod v2 API
This driver is a copy of the existing linuxgpiod driver but adapted
to the newer v2 API of libgpiod.
I've chosen to add it in parallel to the existing driver so that it
is possible to have releases with support for both.
I decided to make a dedicated new module to prevent having a lot of
ifdef-ery inside the C file itself - in my eyes, the small amount of
additionl makefile magic is simpler.
The version of libgpiod is detected during configure time based on
the pkg-config version detection since the library itself does not
provide any defines or similar stuff.
Change-Id: Ifac92b282ff8e83b2aebbcd75b61a554c9d36937
Signed-off-by: Michael Heimpold <m...@heimpold.de>
diff --git a/configure.ac b/configure.ac
index c9cf0214e3..2bde46a03c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -669,7 +669,19 @@ PKG_CHECK_MODULES([LIBFTDI], [libftdi1], [
PKG_CHECK_MODULES([LIBFTDI], [libftdi], [use_libftdi=yes],
[use_libftdi=no])
])
-PKG_CHECK_MODULES([LIBGPIOD], [libgpiod], [use_libgpiod=yes],
[use_libgpiod=no])
+PKG_CHECK_MODULES([LIBGPIOD], [libgpiod >= 2.0], [
+ use_libgpiod=yes
+ is_libgpiodv2=yes
+], [
+ PKG_CHECK_MODULES([LIBGPIOD], [libgpiod], [
+ use_libgpiod=yes
+ is_libgpiodv2=no
+ ], [
+ use_libgpiod=no
+ is_libgpiodv2=no
+ ])
+])
+AM_CONDITIONAL([LINUXGPIODV2], [test x$is_libgpiodv2 = xyes])
PKG_CHECK_MODULES([LIBJAYLINK], [libjaylink >= 0.2],
[use_libjaylink=yes], [use_libjaylink=no])
diff --git a/src/jtag/drivers/Makefile.am b/src/jtag/drivers/Makefile.am
index e404afe9f0..1b12af248a 100644
--- a/src/jtag/drivers/Makefile.am
+++ b/src/jtag/drivers/Makefile.am
@@ -74,8 +74,12 @@ if FTDI
DRIVERFILES += %D%/ftdi.c %D%/mpsse.c
endif
if LINUXGPIOD
+if LINUXGPIODV2
+DRIVERFILES += %D%/linuxgpiodv2.c
+else
DRIVERFILES += %D%/linuxgpiod.c
endif
+endif
if JTAG_VPI
DRIVERFILES += %D%/jtag_vpi.c
endif
diff --git a/src/jtag/drivers/linuxgpiodv2.c b/src/jtag/drivers/linuxgpiodv2.c
index 9428837883..3e2e7607c9 100644
--- a/src/jtag/drivers/linuxgpiodv2.c
+++ b/src/jtag/drivers/linuxgpiodv2.c
@@ -1,9 +1,13 @@
// SPDX-License-Identifier: GPL-2.0-or-later
/*
* Bitbang driver for Linux GPIO descriptors through libgpiod
+ * using the library's v2 API.
+ * Copyright (C) 2024 Michael Heimpold <m...@heimpold.de>
+ *
+ * Derived from linuxgpiod.c:
* Copyright (C) 2020 Antonio Borneo <borneo.anto...@gmail.com>
*
- * Largely based on sysfsgpio driver
+ * Which in turn was largely based on sysfsgpio driver:
* Copyright (C) 2012 by Creative Product Design, marc @ cpdesign.com.au
* Copyright (C) 2014 by Jean-Christian de Rivaz <j...@eclis.ch>
* Copyright (C) 2014 by Paul Fertser <fercer...@gmail.com>
@@ -20,7 +24,9 @@
#include "bitbang.h"
static struct gpiod_chip *gpiod_chip[ADAPTER_GPIO_IDX_NUM] = {};
-static struct gpiod_line *gpiod_line[ADAPTER_GPIO_IDX_NUM] = {};
+static struct gpiod_line_settings *gpiod_line_settings[ADAPTER_GPIO_IDX_NUM] =
{};
+static struct gpiod_line_config *gpiod_line_config[ADAPTER_GPIO_IDX_NUM] = {};
+static struct gpiod_line_request *gpiod_line_request[ADAPTER_GPIO_IDX_NUM] =
{};
static int last_swclk;
static int last_swdio;
@@ -29,6 +35,20 @@ static bool swdio_input;
static const struct adapter_gpio_config *adapter_gpio_config;
+/* Helper to get/set a single line */
+static int linuxgpiod_line_get_value(enum adapter_gpio_config_index idx)
+{
+ return gpiod_line_request_get_value(gpiod_line_request[idx],
+ adapter_gpio_config[idx].gpio_num);
+}
+
+static int linuxgpiod_line_set_value(enum adapter_gpio_config_index idx, int
value)
+{
+ return gpiod_line_request_set_value(gpiod_line_request[idx],
+ adapter_gpio_config[idx].gpio_num,
+ value);
+}
+
/*
* Helper function to determine if gpio config is valid
*
@@ -46,7 +66,7 @@ static bb_value_t linuxgpiod_read(void)
{
int retval;
- retval = gpiod_line_get_value(gpiod_line[ADAPTER_GPIO_IDX_TDO]);
+ retval = linuxgpiod_line_get_value(ADAPTER_GPIO_IDX_TDO);
if (retval < 0) {
LOG_WARNING("reading tdo failed");
return 0;
@@ -79,20 +99,20 @@ static int linuxgpiod_write(int tck, int tms, int tdi)
}
if (tdi != last_tdi) {
- retval = gpiod_line_set_value(gpiod_line[ADAPTER_GPIO_IDX_TDI],
tdi);
+ retval = linuxgpiod_line_set_value(ADAPTER_GPIO_IDX_TDI, tdi);
if (retval < 0)
LOG_WARNING("writing tdi failed");
}
if (tms != last_tms) {
- retval = gpiod_line_set_value(gpiod_line[ADAPTER_GPIO_IDX_TMS],
tms);
+ retval = linuxgpiod_line_set_value(ADAPTER_GPIO_IDX_TMS, tms);
if (retval < 0)
LOG_WARNING("writing tms failed");
}
/* write clk last */
if (tck != last_tck) {
- retval = gpiod_line_set_value(gpiod_line[ADAPTER_GPIO_IDX_TCK],
tck);
+ retval = linuxgpiod_line_set_value(ADAPTER_GPIO_IDX_TCK, tck);
if (retval < 0)
LOG_WARNING("writing tck failed");
}
@@ -108,7 +128,7 @@ static int linuxgpiod_swdio_read(void)
{
int retval;
- retval = gpiod_line_get_value(gpiod_line[ADAPTER_GPIO_IDX_SWDIO]);
+ retval = linuxgpiod_line_get_value(ADAPTER_GPIO_IDX_SWDIO);
if (retval < 0) {
LOG_WARNING("Fail read swdio");
return 0;
@@ -121,30 +141,54 @@ static void linuxgpiod_swdio_drive(bool is_output)
{
int retval;
- /*
- * FIXME: change direction requires release and re-require the line
- * https://stackoverflow.com/questions/58735140/
- * this would change in future libgpiod
- */
- gpiod_line_release(gpiod_line[ADAPTER_GPIO_IDX_SWDIO]);
-
if (is_output) {
- if (gpiod_line[ADAPTER_GPIO_IDX_SWDIO_DIR]) {
- retval =
gpiod_line_set_value(gpiod_line[ADAPTER_GPIO_IDX_SWDIO_DIR], 1);
+ if (gpiod_line_request[ADAPTER_GPIO_IDX_SWDIO_DIR]) {
+ retval =
linuxgpiod_line_set_value(ADAPTER_GPIO_IDX_SWDIO_DIR, 1);
if (retval < 0)
- LOG_WARNING("Fail set swdio_dir");
+ LOG_WARNING("Failed to set swdio_dir=1");
}
- retval =
gpiod_line_request_output(gpiod_line[ADAPTER_GPIO_IDX_SWDIO], "OpenOCD", 1);
+
+ retval =
gpiod_line_settings_set_direction(gpiod_line_settings[ADAPTER_GPIO_IDX_SWDIO],
+ GPIOD_LINE_DIRECTION_OUTPUT);
if (retval < 0)
- LOG_WARNING("Fail request_output line swdio");
+ LOG_WARNING("Failed to set new direction of swdio");
+
+ retval =
gpiod_line_settings_set_output_value(gpiod_line_settings[ADAPTER_GPIO_IDX_SWDIO],
+ GPIOD_LINE_VALUE_ACTIVE);
+ if (retval < 0)
+ LOG_WARNING("Failed to set output value of swdio");
+
+ retval =
gpiod_line_config_add_line_settings(gpiod_line_config[ADAPTER_GPIO_IDX_SWDIO],
+
&adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num, 1,
+ gpiod_line_settings[ADAPTER_GPIO_IDX_SWDIO]);
+ if (retval < 0)
+ LOG_WARNING("Failed to apply output configuration to
swdio");
+
+ retval =
gpiod_line_request_reconfigure_lines(gpiod_line_request[ADAPTER_GPIO_IDX_SWDIO],
+ gpiod_line_config[ADAPTER_GPIO_IDX_SWDIO]);
+ if (retval < 0)
+ LOG_WARNING("Failed to switch swdio to output");
} else {
- retval =
gpiod_line_request_input(gpiod_line[ADAPTER_GPIO_IDX_SWDIO], "OpenOCD");
+ retval =
gpiod_line_settings_set_direction(gpiod_line_settings[ADAPTER_GPIO_IDX_SWDIO],
+ GPIOD_LINE_DIRECTION_INPUT);
+ if (retval < 0)
+ LOG_WARNING("Failed to switch swdio to output");
+
+ retval =
gpiod_line_config_add_line_settings(gpiod_line_config[ADAPTER_GPIO_IDX_SWDIO],
+
&adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num, 1,
+ gpiod_line_settings[ADAPTER_GPIO_IDX_SWDIO]);
+ if (retval < 0)
+ LOG_WARNING("Failed to apply input configuration to
swdio");
+
+ retval =
gpiod_line_request_reconfigure_lines(gpiod_line_request[ADAPTER_GPIO_IDX_SWDIO],
+ gpiod_line_config[ADAPTER_GPIO_IDX_SWDIO]);
if (retval < 0)
- LOG_WARNING("Fail request_input line swdio");
- if (gpiod_line[ADAPTER_GPIO_IDX_SWDIO_DIR]) {
- retval =
gpiod_line_set_value(gpiod_line[ADAPTER_GPIO_IDX_SWDIO_DIR], 0);
+ LOG_WARNING("Failed to switch swdio to input");
+
+ if (gpiod_line_request[ADAPTER_GPIO_IDX_SWDIO_DIR]) {
+ retval =
linuxgpiod_line_set_value(ADAPTER_GPIO_IDX_SWDIO_DIR, 0);
if (retval < 0)
- LOG_WARNING("Fail set swdio_dir");
+ LOG_WARNING("Failed to set swdio_dir=0");
}
}
@@ -158,7 +202,7 @@ static int linuxgpiod_swd_write(int swclk, int swdio)
if (!swdio_input) {
if (!last_stored || (swdio != last_swdio)) {
- retval =
gpiod_line_set_value(gpiod_line[ADAPTER_GPIO_IDX_SWDIO], swdio);
+ retval =
linuxgpiod_line_set_value(ADAPTER_GPIO_IDX_SWDIO, swdio);
if (retval < 0)
LOG_WARNING("Fail set swdio");
}
@@ -166,7 +210,7 @@ static int linuxgpiod_swd_write(int swclk, int swdio)
/* write swclk last */
if (!last_stored || (swclk != last_swclk)) {
- retval =
gpiod_line_set_value(gpiod_line[ADAPTER_GPIO_IDX_SWCLK], swclk);
+ retval = linuxgpiod_line_set_value(ADAPTER_GPIO_IDX_SWCLK,
swclk);
if (retval < 0)
LOG_WARNING("Fail set swclk");
}
@@ -185,7 +229,7 @@ static int linuxgpiod_blink(int on)
if (!is_gpio_config_valid(ADAPTER_GPIO_IDX_LED))
return ERROR_OK;
- retval = gpiod_line_set_value(gpiod_line[ADAPTER_GPIO_IDX_LED], on);
+ retval = linuxgpiod_line_set_value(ADAPTER_GPIO_IDX_LED, on);
if (retval < 0)
LOG_WARNING("Fail set led");
return retval;
@@ -212,17 +256,17 @@ static int linuxgpiod_reset(int trst, int srst)
LOG_DEBUG("linuxgpiod_reset");
/*
- * active low behaviour handled by "adaptor gpio" command and
- * GPIOD_LINE_REQUEST_FLAG_ACTIVE_LOW flag when requesting the line.
+ * active low behavior handled by "adaptor gpio" command and
+ * thus handled when requesting the line.
*/
- if (gpiod_line[ADAPTER_GPIO_IDX_SRST]) {
- retval1 =
gpiod_line_set_value(gpiod_line[ADAPTER_GPIO_IDX_SRST], srst);
+ if (gpiod_line_request[ADAPTER_GPIO_IDX_SRST]) {
+ retval1 = linuxgpiod_line_set_value(ADAPTER_GPIO_IDX_SRST,
srst);
if (retval1 < 0)
LOG_WARNING("set srst value failed");
}
- if (gpiod_line[ADAPTER_GPIO_IDX_TRST]) {
- retval2 =
gpiod_line_set_value(gpiod_line[ADAPTER_GPIO_IDX_TRST], trst);
+ if (gpiod_line_request[ADAPTER_GPIO_IDX_TRST]) {
+ retval2 = linuxgpiod_line_set_value(ADAPTER_GPIO_IDX_TRST,
trst);
if (retval2 < 0)
LOG_WARNING("set trst value failed");
}
@@ -254,9 +298,17 @@ static bool linuxgpiod_swd_mode_possible(void)
static inline void helper_release(enum adapter_gpio_config_index idx)
{
- if (gpiod_line[idx]) {
- gpiod_line_release(gpiod_line[idx]);
- gpiod_line[idx] = NULL;
+ if (gpiod_line_request[idx]) {
+ gpiod_line_request_release(gpiod_line_request[idx]);
+ gpiod_line_request[idx] = NULL;
+ }
+ if (gpiod_line_config[idx]) {
+ gpiod_line_config_free(gpiod_line_config[idx]);
+ gpiod_line_config[idx] = NULL;
+ }
+ if (gpiod_line_settings[idx]) {
+ gpiod_line_settings_free(gpiod_line_settings[idx]);
+ gpiod_line_settings[idx] = NULL;
}
if (gpiod_chip[idx]) {
gpiod_chip_close(gpiod_chip[idx]);
@@ -273,13 +325,26 @@ static int linuxgpiod_quit(void)
return ERROR_OK;
}
+static struct gpiod_chip *gpiod_chip_open_by_number(unsigned int chip_num)
+{
+ char chip_path[32];
+ int l;
+
+ l = snprintf(chip_path, sizeof(chip_path), "/dev/gpiochip%u", chip_num);
+ if (l < 0 || l >= (int)sizeof(chip_path))
+ return NULL;
+
+ return gpiod_chip_open(chip_path);
+}
+
static int helper_get_line(enum adapter_gpio_config_index idx)
{
+ struct gpiod_request_config *req_cfg = NULL;
+ int retval = -1;
+
if (!is_gpio_config_valid(idx))
return ERROR_OK;
- int dir = GPIOD_LINE_REQUEST_DIRECTION_INPUT, flags = 0, val = 0,
retval;
-
gpiod_chip[idx] =
gpiod_chip_open_by_number(adapter_gpio_config[idx].chip_num);
if (!gpiod_chip[idx]) {
LOG_ERROR("Cannot open LinuxGPIOD chip %d for %s",
adapter_gpio_config[idx].chip_num,
@@ -287,72 +352,83 @@ static int helper_get_line(enum adapter_gpio_config_index
idx)
return ERROR_JTAG_INIT_FAILED;
}
- gpiod_line[idx] = gpiod_chip_get_line(gpiod_chip[idx],
adapter_gpio_config[idx].gpio_num);
- if (!gpiod_line[idx]) {
- LOG_ERROR("Error get line %s", adapter_gpio_get_name(idx));
+ gpiod_line_settings[idx] = gpiod_line_settings_new();
+ gpiod_line_config[idx] = gpiod_line_config_new();
+ req_cfg = gpiod_request_config_new();
+
+ if (!gpiod_line_settings[idx] || !gpiod_line_config[idx] || !req_cfg) {
+ LOG_ERROR("Cannot configure LinuxGPIOD line for %s",
adapter_gpio_get_name(idx));
return ERROR_JTAG_INIT_FAILED;
}
+ gpiod_request_config_set_consumer(req_cfg, "OpenOCD");
+
switch (adapter_gpio_config[idx].init_state) {
case ADAPTER_GPIO_INIT_STATE_INPUT:
- dir = GPIOD_LINE_REQUEST_DIRECTION_INPUT;
+ gpiod_line_settings_set_direction(gpiod_line_settings[idx],
GPIOD_LINE_DIRECTION_INPUT);
break;
case ADAPTER_GPIO_INIT_STATE_INACTIVE:
- dir = GPIOD_LINE_REQUEST_DIRECTION_OUTPUT;
- val = 0;
+ gpiod_line_settings_set_direction(gpiod_line_settings[idx],
GPIOD_LINE_DIRECTION_OUTPUT);
+ gpiod_line_settings_set_output_value(gpiod_line_settings[idx],
GPIOD_LINE_VALUE_INACTIVE);
break;
case ADAPTER_GPIO_INIT_STATE_ACTIVE:
- dir = GPIOD_LINE_REQUEST_DIRECTION_OUTPUT;
- val = 1;
+ gpiod_line_settings_set_direction(gpiod_line_settings[idx],
GPIOD_LINE_DIRECTION_OUTPUT);
+ gpiod_line_settings_set_output_value(gpiod_line_settings[idx],
GPIOD_LINE_VALUE_ACTIVE);
break;
}
- switch (adapter_gpio_config[idx].drive) {
- case ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL:
- break;
- case ADAPTER_GPIO_DRIVE_MODE_OPEN_DRAIN:
- flags |= GPIOD_LINE_REQUEST_FLAG_OPEN_DRAIN;
- break;
- case ADAPTER_GPIO_DRIVE_MODE_OPEN_SOURCE:
- flags |= GPIOD_LINE_REQUEST_FLAG_OPEN_SOURCE;
- break;
+ if (adapter_gpio_config[idx].init_state !=
ADAPTER_GPIO_INIT_STATE_INPUT) {
+ switch (adapter_gpio_config[idx].drive) {
+ case ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL:
+ gpiod_line_settings_set_drive(gpiod_line_settings[idx],
GPIOD_LINE_DRIVE_PUSH_PULL);
+ break;
+ case ADAPTER_GPIO_DRIVE_MODE_OPEN_DRAIN:
+ gpiod_line_settings_set_drive(gpiod_line_settings[idx],
GPIOD_LINE_DRIVE_OPEN_DRAIN);
+ break;
+ case ADAPTER_GPIO_DRIVE_MODE_OPEN_SOURCE:
+ gpiod_line_settings_set_drive(gpiod_line_settings[idx],
GPIOD_LINE_DRIVE_OPEN_SOURCE);
+ break;
+ }
}
switch (adapter_gpio_config[idx].pull) {
case ADAPTER_GPIO_PULL_NONE:
-#ifdef GPIOD_LINE_REQUEST_FLAG_BIAS_DISABLE
- flags |= GPIOD_LINE_REQUEST_FLAG_BIAS_DISABLE;
+#ifdef GPIOD_LINE_BIAS_DISABLED
+ gpiod_line_settings_set_bias(gpiod_line_settings[idx],
GPIOD_LINE_BIAS_DISABLED);
#endif
break;
case ADAPTER_GPIO_PULL_UP:
-#ifdef GPIOD_LINE_REQUEST_FLAG_BIAS_PULL_UP
- flags |= GPIOD_LINE_REQUEST_FLAG_BIAS_PULL_UP;
+#ifdef GPIOD_LINE_BIAS_PULL_UP
+ gpiod_line_settings_set_bias(gpiod_line_settings[idx],
GPIOD_LINE_BIAS_PULL_UP);
#else
- LOG_WARNING("linuxgpiod: ignoring request for pull-up on %s:
not supported by gpiod v%s",
- adapter_gpio_get_name(idx), gpiod_version_string());
+ LOG_WARNING("linuxgpiodv2: ignoring request for pull-up on %s:
not supported by gpiod v%s",
+ adapter_gpio_get_name(idx), gpiod_api_version());
#endif
break;
case ADAPTER_GPIO_PULL_DOWN:
-#ifdef GPIOD_LINE_REQUEST_FLAG_BIAS_PULL_DOWN
- flags |= GPIOD_LINE_REQUEST_FLAG_BIAS_PULL_DOWN;
+#ifdef GPIOD_LINE_BIAS_PULL_DOWN
+ gpiod_line_settings_set_bias(gpiod_line_settings[idx],
GPIOD_LINE_BIAS_PULL_DOWN);
#else
- LOG_WARNING("linuxgpiod: ignoring request for pull-down on %s:
not supported by gpiod v%s",
- adapter_gpio_get_name(idx), gpiod_version_string());
+ LOG_WARNING("linuxgpiodv2: ignoring request for pull-down on
%s: not supported by gpiod v%s",
+ adapter_gpio_get_name(idx), gpiod_api_version());
#endif
break;
}
- if (adapter_gpio_config[idx].active_low)
- flags |= GPIOD_LINE_REQUEST_FLAG_ACTIVE_LOW;
+ gpiod_line_settings_set_active_low(gpiod_line_settings[idx],
adapter_gpio_config[idx].active_low);
- struct gpiod_line_request_config config = {
- .consumer = "OpenOCD",
- .request_type = dir,
- .flags = flags,
- };
-
- retval = gpiod_line_request(gpiod_line[idx], &config, val);
+ retval = gpiod_line_config_add_line_settings(gpiod_line_config[idx],
+ &adapter_gpio_config[idx].gpio_num, 1,
gpiod_line_settings[idx]);
if (retval < 0) {
+ gpiod_request_config_free(req_cfg);
+ LOG_ERROR("Error configuring gpio line %s",
adapter_gpio_get_name(idx));
+ return ERROR_JTAG_INIT_FAILED;
+ }
+
+ gpiod_line_request[idx] = gpiod_chip_request_lines(gpiod_chip[idx],
req_cfg, gpiod_line_config[idx]);
+ gpiod_request_config_free(req_cfg);
+
+ if (!gpiod_line_request[idx]) {
LOG_ERROR("Error requesting gpio line %s",
adapter_gpio_get_name(idx));
return ERROR_JTAG_INIT_FAILED;
}
@@ -362,7 +438,7 @@ static int helper_get_line(enum adapter_gpio_config_index
idx)
static int linuxgpiod_init(void)
{
- LOG_INFO("Linux GPIOD JTAG/SWD bitbang driver");
+ LOG_INFO("Linux GPIOD JTAG/SWD bitbang driver (libgpiod v2)");
bitbang_interface = &linuxgpiod_bitbang;
adapter_gpio_config = adapter_gpio_get_config();
--
