Re: [PATCH v3 1/5] tools/gpio: add gpio basic opereations

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Wed, Aug 31, 2016 at 2:45 AM,  <bamvor.zhangjian@xxxxxxxxxx> wrote:

> +int gpio_request(const char *device_name, unsigned int *lines,
> +                unsigned int nlines, unsigned int flag,
> +                struct gpiohandle_data *data, const char *consumer_label)

Could it be named
gpiotools_request_linehandle()?

Or the name is maybe a bit long.

It's nice if we have a name that tells all the function is doing.

> +int gpio_set_values(const int fd, struct gpiohandle_data *data)

Pls rename
gpiotools_set_values()

> +int gpio_get_values(const int fd, struct gpiohandle_data *data)

Pls rename
gpiotools_get_values()

> +int gpio_release(const int fd)

Pls rename
gpiotools_release_linehandle()

> +int gpio_gets(const char *device_name, unsigned int *lines, unsigned int nlines,
> +             unsigned int flag, struct gpiohandle_data *data)
> +int gpio_sets(const char *device_name, unsigned int *lines, unsigned int nlines,
> +             unsigned int flag, struct gpiohandle_data *data)
> +int gpio_get(const char *device_name, unsigned int line, unsigned int flag)
> +int gpio_set(const char *device_name, unsigned int line, unsigned int flag,
> +            unsigned int value)
> +int gpio_set_flag(const char *device_name, unsigned int line, unsigned int flag)

Prefix all gpiotools_*

> +/*
> + * Request the lines for gpio with device_name. Could set the default value
> + * in request.
> + * device_name:    the name of gpiochip in "/dev", such as gpiochip0.
> + * lines:         the array of which line should be requested.
> + * nline:          the total number of line
> + * flag:           input, output and so on. Reference "linux/gpio.h" for the
> + *                 meaning for flag.
> + * data:          default value when flag is GPIOHANDLE_REQUEST_OUTPUT.
> + * consumer_label: the name of consumer, such as "sysfs", "powerkey".
> + *
> + * Return value:   On success return the fd of specific gpiochip. It could be
> + *                 release by gpio_release.
> + *                On failure return the errno.
> + */
> +int gpio_request(const char *device_name, unsigned int *lines,
> +                unsigned int nlines, unsigned int flag,
> +                struct gpiohandle_data *data, const char *consumer_label);

For all of those, convert to kerneldoc (Documentation/kernel-doc-nano-HOWTO)
and move the kerneldoc above the actual implementation, keep the
header file spartan.

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