[PATCH 0/4] Add compatibility with libgpiod v2.0 and higher

[Boerge Struempfel]

This patch series aims to adapt the ucfw handler for use with
libgpiod v2 while still keeping the compatibility with libgpiod v1.

Boerge Struempfel (4):
ucfw: Refactored switch_mode signature
ucfw: Save the gpio chip and lines in gpio structure
ucfw: Refactored gpio handling
ucfw: Add compatibility with libgpiod v2 api

handlers/ucfw_handler.c | 242 +++++++++++++++++++++++++++++++++++-----
1 file changed, 216 insertions(+), 26 deletions(-)

--
2.42.0

[Boerge Struempfel]

This commit aims to reduce the number of parameters, which have to be
given to the switch_mode function. This makes it much easier to add a
different variation of this function depending on the installed libgpiod
version.

Signed-off-by: Boerge Struempfel <boerge.s...@gmail.com>
---
handlers/ucfw_handler.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/handlers/ucfw_handler.c b/handlers/ucfw_handler.c
index 8d10a54..5bf23c0 100644
--- a/handlers/ucfw_handler.c
+++ b/handlers/ucfw_handler.c
@@ -109,16 +109,17 @@ struct handler_priv {
unsigned int nbytes;
};

-static int switch_mode(char *devreset, int resoffset, char *devprog, int progoffset, int mode)
+
+static int switch_mode(struct handler_priv *priv, int mode)
{
struct gpiod_chip *chipreset, *chipprog;
struct gpiod_line *linereset, *lineprog;
int ret = 0;
int status;

- chipreset = gpiod_chip_open(devreset);
- if (strcmp(devreset, devprog))
- chipprog = gpiod_chip_open(devprog);
+ chipreset = gpiod_chip_open(priv->reset.gpiodev);
+ if (strcmp(priv->reset.gpiodev, priv->prog.gpiodev))
+ chipprog = gpiod_chip_open(priv->prog.gpiodev);
else
chipprog = chipreset;

@@ -128,13 +129,13 @@ static int switch_mode(char *devreset, int resoffset, char *devprog, int progoff
goto freegpios;
}

- linereset = gpiod_chip_get_line(chipreset, resoffset);
- lineprog = gpiod_chip_get_line(chipprog, progoffset);
+ linereset = gpiod_chip_get_line(chipreset, priv->reset.offset);
+ lineprog = gpiod_chip_get_line(chipprog, priv->prog.offset);

if (!linereset || !lineprog) {
ERROR("Cannot get requested GPIOs: %d on %s and %d on %s",
- resoffset, devreset,
- progoffset, devprog);
+ priv->reset.offset, priv->reset.gpiodev,
+ priv->prog.offset, priv->prog.gpiodev);
ret =-ENODEV;
goto freegpios;
}
@@ -378,8 +379,7 @@ static int prepare_update(struct handler_priv *priv,
char msg[128];
int len;

- ret = switch_mode(priv->reset.gpiodev, priv->reset.offset,
- priv->prog.gpiodev, priv->prog.offset, MODE_PROG);
+ ret = switch_mode(priv, MODE_PROG);
if (ret < 0) {
return -ENODEV;
}
--
2.42.0

[Boerge Struempfel]

This is a prerequisite for splitting the switch_mode function into a
the initialization phase, the actual mode-setting phase and then a
cleanup phase.

Signed-off-by: Boerge Struempfel <boerge.s...@gmail.com>
---

handlers/ucfw_handler.c | 43 ++++++++++++++++++++++-------------------
1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/handlers/ucfw_handler.c b/handlers/ucfw_handler.c
index 5bf23c0..6b986f5 100644
--- a/handlers/ucfw_handler.c
+++ b/handlers/ucfw_handler.c
@@ -91,6 +91,8 @@ struct mode_setup {
char gpiodev[SWUPDATE_GENERAL_STRING_SIZE];
unsigned int offset;
bool active_low;
+ struct gpiod_chip *chip;
+ struct gpiod_line *line;
};

enum {
@@ -109,30 +111,27 @@ struct handler_priv {
unsigned int nbytes;
};

-

static int switch_mode(struct handler_priv *priv, int mode)
{

- struct gpiod_chip *chipreset, *chipprog;
- struct gpiod_line *linereset, *lineprog;

int ret = 0;
int status;

- chipreset = gpiod_chip_open(priv->reset.gpiodev);
+ priv->reset.chip = gpiod_chip_open(priv->reset.gpiodev);

if (strcmp(priv->reset.gpiodev, priv->prog.gpiodev))

- chipprog = gpiod_chip_open(priv->prog.gpiodev);
+ priv->prog.chip = gpiod_chip_open(priv->prog.gpiodev);
else
- chipprog = chipreset;
+ priv->prog.chip = priv->reset.chip;

- if (!chipreset || !chipprog) {
+ if (!priv->reset.chip || !priv->prog.chip) {
ERROR("Cannot open gpio driver");
ret =-ENODEV;
goto freegpios;
}

- linereset = gpiod_chip_get_line(chipreset, priv->reset.offset);
- lineprog = gpiod_chip_get_line(chipprog, priv->prog.offset);
+ priv->reset.line = gpiod_chip_get_line(priv->reset.chip, priv->reset.offset);
+ priv->prog.line = gpiod_chip_get_line(priv->prog.chip, priv->prog.offset);

- if (!linereset || !lineprog) {
+ if (!priv->reset.line || !priv->prog.line) {

ERROR("Cannot get requested GPIOs: %d on %s and %d on %s",

priv->reset.offset, priv->reset.gpiodev,
priv->prog.offset, priv->prog.gpiodev);
@@ -140,13 +139,13 @@ static int switch_mode(struct handler_priv *priv, int mode)
goto freegpios;
}

- status = gpiod_line_request_output(linereset, RESET_CONSUMER, 0);
+ status = gpiod_line_request_output(priv->reset.line, RESET_CONSUMER, 0);
if (status) {
ret =-ENODEV;
ERROR("Cannot request reset line");
goto freegpios;
}
- status = gpiod_line_request_output(lineprog, PROG_CONSUMER, mode);
+ status = gpiod_line_request_output(priv->prog.line, PROG_CONSUMER, mode);
if (status) {
ret =-ENODEV;
ERROR("Cannot request prog line");
@@ -156,22 +155,27 @@ static int switch_mode(struct handler_priv *priv, int mode)
/*
* A reset is always done
*/
- gpiod_line_set_value(linereset, 0);
+ gpiod_line_set_value(priv->reset.line, 0);

/* Set programming mode */
- gpiod_line_set_value(lineprog, mode);
+ gpiod_line_set_value(priv->prog.line, mode);

usleep(20000);

/* Remove reset */
- gpiod_line_set_value(linereset, 1);
+ gpiod_line_set_value(priv->reset.line, 1);

usleep(20000);

freegpios:
- if (chipreset) gpiod_chip_close(chipreset);
- if (chipprog && (chipprog != chipreset)) gpiod_chip_close(chipprog);
-
+ if (priv->prog.chip && (priv->prog.chip != priv->reset.chip)){
+ gpiod_chip_close(priv->prog.chip);
+ priv->reset.chip = NULL;
+ }
+ if (priv->reset.chip) {
+ gpiod_chip_close(priv->reset.chip);
+ priv->reset.chip = NULL;
+ }
return ret;
}

@@ -449,8 +453,7 @@ static int finish_update(struct handler_priv *priv)
int ret;

close(priv->fduart);

- ret = switch_mode(priv->reset.gpiodev, priv->reset.offset,

- priv->prog.gpiodev, priv->prog.offset, MODE_NORMAL);
+ ret = switch_mode(priv, MODE_NORMAL);

[Boerge Struempfel]

This commit splits the switch_mode function into the
gpio-initialization, the actual mode switch and the cleanup phases. This
is mainly done in order to prepare for adding gpiolib v2 api capability
but has the added advantage, that the gpios are no longer released
during the update, which for one prevents the gpios from floating, and
for another, stops other processes from taking over the pins during an
update phase.

Signed-off-by: Boerge Struempfel <boerge.s...@gmail.com>
---

handlers/ucfw_handler.c | 60 ++++++++++++++++++++++++++++++-----------
1 file changed, 45 insertions(+), 15 deletions(-)

diff --git a/handlers/ucfw_handler.c b/handlers/ucfw_handler.c
index 6b986f5..0296f17 100644
--- a/handlers/ucfw_handler.c
+++ b/handlers/ucfw_handler.c
@@ -111,8 +111,18 @@ struct handler_priv {
unsigned int nbytes;
};

-static int switch_mode(struct handler_priv *priv, int mode)
-{
+static void free_gpios(struct handler_priv *priv) {

+ if (priv->prog.chip && (priv->prog.chip != priv->reset.chip)){
+ gpiod_chip_close(priv->prog.chip);
+ priv->reset.chip = NULL;
+ }
+ if (priv->reset.chip) {
+ gpiod_chip_close(priv->reset.chip);
+ priv->reset.chip = NULL;
+ }

+}
+
+static int register_gpios(struct handler_priv *priv){

int ret = 0;
int status;

@@ -145,37 +155,51 @@ static int switch_mode(struct handler_priv *priv, int mode)

ERROR("Cannot request reset line");
goto freegpios;
}

- status = gpiod_line_request_output(priv->prog.line, PROG_CONSUMER, mode);
+ status = gpiod_line_request_output(priv->prog.line, PROG_CONSUMER, 0);
if (status) {
ret =-ENODEV;
ERROR("Cannot request prog line");
goto freegpios;
}

+ return ret;
+freegpios:
+ free_gpios(priv);
+ return ret;
+}
+
+static int switch_mode(struct handler_priv *priv, int mode)
+{
+ int ret = 0;
+ if (!priv->reset.line || !priv->prog.line) return -ENODEV;
+

/*
* A reset is always done
*/

- gpiod_line_set_value(priv->reset.line, 0);
+ ret = gpiod_line_set_value(priv->reset.line, 0);
+ if (ret){
+ ERROR("unable to set reset to 0");
+ return ret;
+ }

/* Set programming mode */

- gpiod_line_set_value(priv->prog.line, mode);
+ ret = gpiod_line_set_value(priv->prog.line, mode);
+ if (ret){
+ ERROR("unable to set prog to %i",mode);
+ return ret;
+ }

usleep(20000);

/* Remove reset */

- gpiod_line_set_value(priv->reset.line, 1);
+ ret = gpiod_line_set_value(priv->reset.line, 1);
+ if (ret){
+ ERROR("unable to set reset to 1");
+ return ret;
+ }

usleep(20000);

-freegpios:
- if (priv->prog.chip && (priv->prog.chip != priv->reset.chip)){
- gpiod_chip_close(priv->prog.chip);
- priv->reset.chip = NULL;
- }
- if (priv->reset.chip) {
- gpiod_chip_close(priv->reset.chip);
- priv->reset.chip = NULL;
- }
return ret;
}

@@ -383,6 +407,11 @@ static int prepare_update(struct handler_priv *priv,

char msg[128];
int len;

+ ret = register_gpios(priv);
+ if (ret < 0) {
+ return -ENODEV;
+ }

+
ret = switch_mode(priv, MODE_PROG);

if (ret < 0) {
return -ENODEV;

@@ -454,6 +483,7 @@ static int finish_update(struct handler_priv *priv)

close(priv->fduart);
ret = switch_mode(priv, MODE_NORMAL);
+ free_gpios(priv);

[Boerge Struempfel]

In order to detect the libgpiod api version, we use
GPIOD_LINE_BULK_MAX_LINES, since this is only defined in v1 and not in
v2.

Signed-off-by: Boerge Struempfel <boerge.s...@gmail.com>
---

handlers/ucfw_handler.c | 157 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 157 insertions(+)

diff --git a/handlers/ucfw_handler.c b/handlers/ucfw_handler.c
index 0296f17..ad33f05 100644
--- a/handlers/ucfw_handler.c
+++ b/handlers/ucfw_handler.c
@@ -80,6 +80,14 @@
#define PROG_CONSUMER RESET_CONSUMER
#define DEFAULT_TIMEOUT 2

+/*
+ * Use GPIOD_LINE_BULK_MAX_LINES in order to determine,
+ * whether this is compiled using libgpio v1 or v2.
+ */
+#ifdef GPIOD_LINE_BULK_MAX_LINES
+#define USE_GPIOD_API_V1
+#endif
+
void ucfw_handler(void);

/*
@@ -91,8 +99,12 @@ struct mode_setup {

char gpiodev[SWUPDATE_GENERAL_STRING_SIZE];
unsigned int offset;
bool active_low;

+#ifdef USE_GPIOD_API_V1
struct gpiod_chip *chip;
struct gpiod_line *line;
+#else
+ struct gpiod_line_request *request;
+#endif
};

enum {
@@ -111,6 +123,7 @@ struct handler_priv {
unsigned int nbytes;
};

+#ifdef USE_GPIOD_API_V1
static void free_gpios(struct handler_priv *priv) {

if (priv->prog.chip && (priv->prog.chip != priv->reset.chip)){

gpiod_chip_close(priv->prog.chip);
@@ -203,6 +216,150 @@ static int switch_mode(struct handler_priv *priv, int mode)
return ret;
}

+#else
+/* Implementation for LIBGPIOD V2 api*/
+

+static void free_gpios(struct handler_priv *priv) {

+ if(priv->reset.request){
+ gpiod_line_request_release(priv->reset.request);
+ priv->reset.request = NULL;
+ }
+ if(priv->prog.request){
+ gpiod_line_request_release(priv->prog.request);
+ priv->prog.request = NULL;

+ }
+}
+
+static int register_gpios(struct handler_priv *priv) {

+ struct gpiod_line_settings *settings;
+ struct gpiod_request_config *req_cfg;
+ struct gpiod_line_config *reset_cfg, *prog_cfg;
+ struct gpiod_chip *chip;

+ int ret = 0;
+

+ settings = gpiod_line_settings_new();
+ if (!settings) {
+ ERROR("Unable to allocate line settings");
+ return -ENODEV;
+ }
+ gpiod_line_settings_set_direction(settings, GPIOD_LINE_DIRECTION_OUTPUT);
+
+ req_cfg = gpiod_request_config_new();
+ if (!req_cfg) {
+ ERROR("Unable to allocate the request config structure");
+ ret =-ENODEV;
+ goto freesettings;
+ }
+ gpiod_request_config_set_consumer(req_cfg, RESET_CONSUMER);
+
+ reset_cfg = gpiod_line_config_new();
+ if (!reset_cfg) {
+ ERROR("Unable to allocate the reset line config structure");
+ ret =-ENODEV;
+ goto freerequestconfig;
+ }
+ ret = gpiod_line_config_add_line_settings(reset_cfg, &priv->reset.offset, 1, settings);
+ if (ret) {
+ ERROR("Unable to add reset line settings");
+ goto freeresetconfig;
+ }
+
+ prog_cfg = gpiod_line_config_new();
+ if (!prog_cfg) {
+ ERROR("Unable to allocate the prog line config structure");
+ ret =-ENODEV;
+ goto freeresetconfig;
+ }
+
+ ret = gpiod_line_config_add_line_settings(prog_cfg, &priv->prog.offset, 1, settings);
+ if (ret) {
+ ERROR("Unable to add reset line settings");
+ goto freeprogconfig;
+ }
+
+ TRACE("Request lines for reset");
+ chip = gpiod_chip_open(priv->reset.gpiodev);
+ if (!chip) {
+ ERROR("Unable to open chip '%s'",priv->reset.gpiodev);
+ ret =-ENODEV;
+ goto freeprogconfig;
+ }
+
+ priv->reset.request = gpiod_chip_request_lines(chip, req_cfg, reset_cfg);
+ gpiod_chip_close(chip);
+ if (!priv->reset.request) {
+ ERROR("Unable to request lines on chip '%s'", priv->reset.gpiodev);
+ goto freeprogconfig;
+ }
+
+ TRACE("Request lines for prog");
+ chip = gpiod_chip_open(priv->prog.gpiodev);
+ if (!chip) {
+ ERROR("Unable to open chip '%s'", priv->prog.gpiodev);
+ ret =-ENODEV;
+ goto freelinerequest;
+ }
+
+ priv->prog.request = gpiod_chip_request_lines(chip, req_cfg, prog_cfg);
+ gpiod_chip_close(chip);
+ if (!priv->prog.request) {
+ ERROR("unable to request lines on chip '%s'", priv->prog.gpiodev);
+ ret =-ENODEV;
+ }
+
+ goto freeprogconfig; // clean up everything except for the gpiod_line_requests
+
+freelinerequest:
+ gpiod_line_request_release(priv->reset.request);
+ priv->reset.request = NULL;
+freeprogconfig:
+ gpiod_line_config_free(prog_cfg);
+freeresetconfig:
+ gpiod_line_config_free(reset_cfg);
+freerequestconfig:
+ gpiod_request_config_free(req_cfg);
+freesettings:
+ gpiod_line_settings_free(settings);

+ return ret;
+}
+
+
+static int switch_mode(struct handler_priv *priv, int mode)
+{
+
+ int ret = 0;
+

+ /*
+ * A reset is always done
+ */
+ ret = gpiod_line_request_set_value(priv->reset.request, priv->reset.offset, 0);
+ if (ret){
+ ERROR("Unable to set reset to 0");
+ return ret;
+ }
+

+ /* Set programming mode */

+ ret = gpiod_line_request_set_value(priv->prog.request, priv->prog.offset, mode);
+ if (ret){
+ ERROR("Unable to set prog to %i", mode);

+ return ret;
+ }
+

+ usleep(20000);
+
+ /* Remove reset */
+ ret = gpiod_line_request_set_value(priv->reset.request, priv->reset.offset, 1);
+ if (ret){
+ ERROR("Unable to set reset to 1");
+ return ret;
+ }
+
+ usleep(20000);
+
+ return ret;
+}
+#endif
+
static bool verify_chksum(char *buf, unsigned int *size)
{
int i;
--
2.42.0

[Stefano Babic]

On 20.10.23 14:33, Boerge Struempfel wrote:
> This patch series aims to adapt the ucfw handler for use with
> libgpiod v2 while still keeping the compatibility with libgpiod v1.
>

Thanks for this works !

It is already the second time that we have to adapt SWUpdate due to
libgpiod changes in API, I have hoped tis interface was more stable..

[Stefano Babic]

On 20.10.23 14:33, Boerge Struempfel wrote:

> This commit aims to reduce the number of parameters, which have to be
> given to the switch_mode function. This makes it much easier to add a
> different variation of this function depending on the installed libgpiod
> version.
>

I checked the code and by reading the API changes in libgpiod, this
series is fine.

However, I have currently no hardware to test it - nice if someone will
test it, and send Tested-by for inclusion.

Nevertheless, I set this series for inclusion, I will just wait a little.

Acked-by: Stefano Babic <stefan...@swupdate.org>

Best regards,
Stefano Babic

[Stefano Babic]

On 20.10.23 14:33, Boerge Struempfel wrote:

Applied to -master, thanks !

Best regards,
Stefano Babic