tools/gpio: add gpio util to get or set gpio lines

Add a new tool to set or get the value of one or more gpio lines, which
can as either chip offsets or line names. This makes it easier to
control gpio lines from user space without using the cumbersome sysfs
interface. It also demostrates how gpio-line-names can be used to
decouple software from hardware layout.

Signed-off-by: Martin Hundebøll <mnhu@prevas.dk>
---
 tools/gpio/.gitignore |   2 +-
 tools/gpio/Build      |   1 +
 tools/gpio/Makefile   |  11 ++-
 tools/gpio/gpio.c     | 261 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 273 insertions(+), 2 deletions(-)
 create mode 100644 tools/gpio/gpio.c

On Mon, Jul 24, 2017 at 4:32 PM, Martin Hundebøll <mnhu@prevas.dk> wrote:

> Add a new tool to set or get the value of one or more gpio lines, which
> can as either chip offsets or line names. This makes it easier to
> control gpio lines from user space without using the cumbersome sysfs
> interface. It also demostrates how gpio-line-names can be used to
> decouple software from hardware layout.
>
> Signed-off-by: Martin Hundebøll <mnhu@prevas.dk>

How does this relate to Bartosz userspace library libgpiod?
Have you checked it and considered using it?
https://github.com/brgl/libgpiod

>  tools/gpio/gpio.c     | 261 ++++++++++++++++++++++++++++++++++++++++++++++++++

We can't have a tool simply named "gpio" it is too unspecific.

Come up with a better name for this, such as "gpiog-get-set" or so.

Maybe it should be two tools? One fo setting and one for getting a line?

> + * lsgpio - example on how to list the GPIO lines on a system
> + *
> + * Copyright (C) 2015 Linus Walleij
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * Usage:
> + *     lsgpio <-n device-name>

There is a lot of copy-paste going on here. Please fix it up.

> +int print_offsets(const char *device_name, unsigned int *lines, int nlines)
> +{
> +       struct gpiohandle_data data;
> +       int i, line, value, ret;
> +
> +       ret = gpiotools_gets(device_name, lines, nlines, &data);
> +       if (ret)
> +               return ret;
> +
> +       /* print value for each configured line */
> +       printf("GPIO chip: %s\n", device_name);
> +       for (i = 0; i < nlines; i++) {
> +               line = lines[i];
> +               value = data.values[line];
> +               printf("\tline %i: %i\n", lines[i], value);
> +       }
> +
> +       return ret;
> +}
> +
> +int print_labels(const char *device_name, unsigned int *lines,
> +                 const char *labels[], int nlabels)
> +{
> +       struct gpiohandle_data data;
> +       int i, value, ret;
> +       const char *label;
> +
> +       ret = gpiotools_gets(device_name, lines, nlabels, &data);
> +       if (ret)
> +               return ret;
> +
> +       /* print value for each configured label */
> +       printf("GPIO chip: %s\n", device_name);
> +       for (i = 0; i < nlabels; i++) {
> +               label = labels[i];
> +               value = data.values[i];
> +               printf("\t%s: %i\n", label, value);
> +       }
> +
> +       return ret;
> +}
> +
> +int control_offsets(const char *device_name, unsigned int *lines, int nlines,
> +                   int flags, int value)
> +{
> +       struct gpiohandle_data data = {{ value }};
> +
> +       if (flags & GPIOHANDLE_REQUEST_INPUT)
> +               return print_offsets(device_name, lines, nlines);
> +       else
> +               return gpiotools_sets(device_name, lines, nlines, &data);
> +}
> +
> +int control_labels(const char *device_name, const char *labels[], int nlabels,
> +                  int flags, int value)
> +{
> +       struct gpiochip_info cinfo = {0};
> +       char *chrdev_name = NULL;
> +       unsigned int lines[GPIOHANDLES_MAX];
> +       int nlines = 0;
> +       int ret, fd, i, j;
> +
> +       ret = asprintf(&chrdev_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\n", chrdev_name);
> +               goto exit_error;
> +       }
> +
> +       /* Inspect this GPIO chip */
> +       ret = ioctl(fd, GPIO_GET_CHIPINFO_IOCTL, &cinfo);
> +       if (ret == -1) {
> +               ret = -errno;
> +               perror("Failed to issue CHIPINFO IOCTL\n");
> +               goto exit_close_error;
> +       }
> +
> +       /* Loop over the lines and match info */
> +       for (i = 0; i < cinfo.lines; i++) {
> +               struct gpioline_info linfo;
> +
> +               memset(&linfo, 0, sizeof(linfo));
> +               linfo.line_offset = i;
> +
> +               ret = ioctl(fd, GPIO_GET_LINEINFO_IOCTL, &linfo);
> +               if (ret == -1) {
> +                       ret = -errno;
> +                       perror("Failed to issue LINEINFO IOCTL\n");
> +                       goto exit_close_error;
> +               }
> +
> +               /* Check if line name is included requested names */
> +               for (j = 0; j < nlabels; j++) {
> +                       if (strcmp(linfo.name, labels[j]) == 0) {
> +                               lines[nlines++] = i;
> +                               break;
> +                       }
> +               }
> +       }
> +
> +       if (nlines == 0)
> +               goto exit_close_error;
> +
> +       if (flags & GPIOHANDLE_REQUEST_INPUT) {
> +               print_labels(device_name, lines, labels, nlabels);
> +       } else {
> +               struct gpiohandle_data data = {{ value }};
> +               ret = gpiotools_sets(device_name, lines, nlines, &data);
> +       }
> +
> +exit_close_error:
> +       if (fd >= 0 && close(fd) == -1)
> +               perror("Failed to close GPIO character device file");
> +
> +exit_error:
> +       free(chrdev_name);
> +
> +       return ret;
> +}

All of this also looks copypasted.

Please go through your program and make something very
specific and to the point that does exactly what you want instead of
randomly reusing lsgpio.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On August 3, 2017 9:56:00 AM GMT+02:00, Linus Walleij <linus.walleij@linaro.org> wrote:
>On Mon, Jul 24, 2017 at 4:32 PM, Martin Hundebøll <mnhu@prevas.dk>
>wrote:
>
>> Add a new tool to set or get the value of one or more gpio lines,
>which
>> can as either chip offsets or line names. This makes it easier to
>> control gpio lines from user space without using the cumbersome sysfs
>> interface. It also demostrates how gpio-line-names can be used to
>> decouple software from hardware layout.
>>
>> Signed-off-by: Martin Hundebøll <mnhu@prevas.dk>
>
>How does this relate to Bartosz userspace library libgpiod?
>Have you checked it and considered using it?
>https://github.com/brgl/libgpiod

I tried with the best of my Google Foo, but "gpio" is a bit too generic. libgpiod is exactly what I need, thanks.

>
>>  tools/gpio/gpio.c     | 261
>++++++++++++++++++++++++++++++++++++++++++++++++++
>
>We can't have a tool simply named "gpio" it is too unspecific.
>
>Come up with a better name for this, such as "gpiog-get-set" or so.
>
>Maybe it should be two tools? One fo setting and one for getting a
>line?

Correct. I can update the patch into two utils, but with libgpiod in the mix, we can just drop this?

// Martin

>
>> + * lsgpio - example on how to list the GPIO lines on a system
>> + *
>> + * Copyright (C) 2015 Linus Walleij
>> + *
>> + * This program is free software; you can redistribute it and/or
>modify it
>> + * under the terms of the GNU General Public License version 2 as
>published by
>> + * the Free Software Foundation.
>> + *
>> + * Usage:
>> + *     lsgpio <-n device-name>
>
>There is a lot of copy-paste going on here. Please fix it up.
>
>> +int print_offsets(const char *device_name, unsigned int *lines, int
>nlines)
>> +{
>> +       struct gpiohandle_data data;
>> +       int i, line, value, ret;
>> +
>> +       ret = gpiotools_gets(device_name, lines, nlines, &data);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* print value for each configured line */
>> +       printf("GPIO chip: %s\n", device_name);
>> +       for (i = 0; i < nlines; i++) {
>> +               line = lines[i];
>> +               value = data.values[line];
>> +               printf("\tline %i: %i\n", lines[i], value);
>> +       }
>> +
>> +       return ret;
>> +}
>> +
>> +int print_labels(const char *device_name, unsigned int *lines,
>> +                 const char *labels[], int nlabels)
>> +{
>> +       struct gpiohandle_data data;
>> +       int i, value, ret;
>> +       const char *label;
>> +
>> +       ret = gpiotools_gets(device_name, lines, nlabels, &data);
>> +       if (ret)
>> +               return ret;
>> +
>> +       /* print value for each configured label */
>> +       printf("GPIO chip: %s\n", device_name);
>> +       for (i = 0; i < nlabels; i++) {
>> +               label = labels[i];
>> +               value = data.values[i];
>> +               printf("\t%s: %i\n", label, value);
>> +       }
>> +
>> +       return ret;
>> +}
>> +
>> +int control_offsets(const char *device_name, unsigned int *lines,
>int nlines,
>> +                   int flags, int value)
>> +{
>> +       struct gpiohandle_data data = {{ value }};
>> +
>> +       if (flags & GPIOHANDLE_REQUEST_INPUT)
>> +               return print_offsets(device_name, lines, nlines);
>> +       else
>> +               return gpiotools_sets(device_name, lines, nlines,
>&data);
>> +}
>> +
>> +int control_labels(const char *device_name, const char *labels[],
>int nlabels,
>> +                  int flags, int value)
>> +{
>> +       struct gpiochip_info cinfo = {0};
>> +       char *chrdev_name = NULL;
>> +       unsigned int lines[GPIOHANDLES_MAX];
>> +       int nlines = 0;
>> +       int ret, fd, i, j;
>> +
>> +       ret = asprintf(&chrdev_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\n", chrdev_name);
>> +               goto exit_error;
>> +       }
>> +
>> +       /* Inspect this GPIO chip */
>> +       ret = ioctl(fd, GPIO_GET_CHIPINFO_IOCTL, &cinfo);
>> +       if (ret == -1) {
>> +               ret = -errno;
>> +               perror("Failed to issue CHIPINFO IOCTL\n");
>> +               goto exit_close_error;
>> +       }
>> +
>> +       /* Loop over the lines and match info */
>> +       for (i = 0; i < cinfo.lines; i++) {
>> +               struct gpioline_info linfo;
>> +
>> +               memset(&linfo, 0, sizeof(linfo));
>> +               linfo.line_offset = i;
>> +
>> +               ret = ioctl(fd, GPIO_GET_LINEINFO_IOCTL, &linfo);
>> +               if (ret == -1) {
>> +                       ret = -errno;
>> +                       perror("Failed to issue LINEINFO IOCTL\n");
>> +                       goto exit_close_error;
>> +               }
>> +
>> +               /* Check if line name is included requested names */
>> +               for (j = 0; j < nlabels; j++) {
>> +                       if (strcmp(linfo.name, labels[j]) == 0) {
>> +                               lines[nlines++] = i;
>> +                               break;
>> +                       }
>> +               }
>> +       }
>> +
>> +       if (nlines == 0)
>> +               goto exit_close_error;
>> +
>> +       if (flags & GPIOHANDLE_REQUEST_INPUT) {
>> +               print_labels(device_name, lines, labels, nlabels);
>> +       } else {
>> +               struct gpiohandle_data data = {{ value }};
>> +               ret = gpiotools_sets(device_name, lines, nlines,
>&data);
>> +       }
>> +
>> +exit_close_error:
>> +       if (fd >= 0 && close(fd) == -1)
>> +               perror("Failed to close GPIO character device file");
>> +
>> +exit_error:
>> +       free(chrdev_name);
>> +
>> +       return ret;
>> +}
>
>All of this also looks copypasted.
>
>Please go through your program and make something very
>specific and to the point that does exactly what you want instead of
>randomly reusing lsgpio.
>
>Yours,
>Linus Walleij

On Thu, Aug 3, 2017 at 10:31 AM, Martin Hundebøll
<martin.hundeboll@prevas.dk> wrote:

>>Maybe it should be two tools? One fo setting and one for getting a
>>line?
>
> Correct. I can update the patch into two utils, but with libgpiod in the mix, we can just drop this?

I would like Bartosz library to be used widely so let's drop this.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 2017-08-07 14:33, Linus Walleij wrote:
> On Thu, Aug 3, 2017 at 10:31 AM, Martin Hundebøll
> <martin.hundeboll@prevas.dk> wrote:
> 
>>> Maybe it should be two tools? One fo setting and one for getting a
>>> line?
>>
>> Correct. I can update the patch into two utils, but with libgpiod in the mix, we can just drop this?
> 
> I would like Bartosz library to be used widely so let's drop this.

Would you accept a patch updating the documentation with a hint about 
tools/gpio and libgpiod?

On Tue, Aug 15, 2017 at 2:54 PM, Martin Hundebøll
<martin.hundeboll@prevas.dk> wrote:

> Would you accept a patch updating the documentation with a hint about
> tools/gpio and libgpiod?

Easily, it would be much appreciated!

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html