gpio: add new SET_CONFIG ioctl() to gpio chardev
Kent Gibson
reviewerv2 cover gpio: add new SET_CONFIG ioctl() to gpio chardev
Changes from v1:
Address v1 review comments, specifically refactoring line handle flag validation out of linehandle_create in a separate patch, and refactoring the set_config itself to improve readability and minimize changes to existing code.
Add the GPIOHANDLE_SET_CONFIG_IOCTL to the gpio chardev. The ioctl allows some of the configuration of a requested handle to be changed without having to release the line.
The primary use case is the changing of direction for bi-directional lines.
The patch series is against Bart's gpio/for-next branch[1] with v4 of my bias patch series "gpio: expose line bias flags to userspace" applied. This is necessary to support setting of the bias flags introduced there.
The patch has been successfully tested against gpio-mockup using the feature/pud_set_config branch of my Go gpiod library[2], as libgpiod has not yet been updated with the uAPI change.
Kent Gibson (2):
gpiolib: move validation of line handle flags into helper function
gpio: add new SET_CONFIG ioctl() to gpio chardev
drivers/gpio/gpiolib.c | 162 ++++++++++++++++++++++++++++---------- include/uapi/linux/gpio.h | 18 +++++ 2 files changed, 138 insertions(+), 42 deletions(-)
Signature · 4 lines
2.23.0 [1] git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git [2] https://github.com/warthog618/gpiod.git
Kent Gibson
reviewerPATCH v2 1/2 gpiolib: move validation of line handle flags into helper function
Move validation of line handle flags into helper function. This reduces the size and complexity of linehandle_create and allows the validation to be reused elsewhere.
--- drivers/gpio/gpiolib.c | 93 +++++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 42 deletions(-)
drivers/gpio/gpiolib.c +51 -42
@@ -427,6 +427,54 @@ struct linehandle_state { GPIOHANDLE_REQUEST_OPEN_DRAIN | \ GPIOHANDLE_REQUEST_OPEN_SOURCE) +static int linehandle_validate_flags (u32 flags) +{ +/* Return an error if an unknown flag is set */ +if (flags & ~GPIOHANDLE_REQUEST_VALID_FLAGS ) +return -EINVAL ; + +/* +* Do not allow both INPUT & OUTPUT flags to be set as they are +* contradictory. +*/ +if ((flags &GPIOHANDLE_REQUEST_INPUT ) && + (flags &GPIOHANDLE_REQUEST_OUTPUT )) +return -EINVAL ; + +/* +* Do not allow OPEN_SOURCE & OPEN_DRAIN flags in a single request. If +* the hardware actually supports enabling both at the same time the +* electrical result would be disastrous. +*/ +if ((flags &GPIOHANDLE_REQUEST_OPEN_DRAIN ) && + (flags &GPIOHANDLE_REQUEST_OPEN_SOURCE )) +return -EINVAL ; + +/* OPEN_DRAIN and OPEN_SOURCE flags only make sense for output mode. */ +if (!(flags &GPIOHANDLE_REQUEST_OUTPUT ) && + ((flags &GPIOHANDLE_REQUEST_OPEN_DRAIN ) || + (flags &GPIOHANDLE_REQUEST_OPEN_SOURCE ))) +return -EINVAL ; + +/* Bias flags only allowed for input or output mode. */ +if (!((flags &GPIOHANDLE_REQUEST_INPUT ) || + (flags &GPIOHANDLE_REQUEST_OUTPUT )) && + ((flags &GPIOHANDLE_REQUEST_BIAS_DISABLE ) || + (flags &GPIOHANDLE_REQUEST_BIAS_PULL_UP ) || + (flags &GPIOHANDLE_REQUEST_BIAS_PULL_DOWN ))) +return -EINVAL ; + +/* Only one bias flag can be set. */ +if (((flags &GPIOHANDLE_REQUEST_BIAS_DISABLE ) && + (flags & (GPIOHANDLE_REQUEST_BIAS_PULL_DOWN | +GPIOHANDLE_REQUEST_BIAS_PULL_UP ))) || + ((flags &GPIOHANDLE_REQUEST_BIAS_PULL_DOWN ) && + (flags &GPIOHANDLE_REQUEST_BIAS_PULL_UP ))) +return -EINVAL ; + +return 0 ; +} +static long linehandle_ioctl (struct file *filep,unsigned int cmd,unsigned long arg) {
@@ -528,48 +576,9 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip) lflags = handlereq.flags; -/* Return an error if an unknown flag is set */ -if (lflags & ~GPIOHANDLE_REQUEST_VALID_FLAGS ) -return -EINVAL ; - -/* -* Do not allow both INPUT & OUTPUT flags to be set as they are -* contradictory. -*/ -if ((lflags &GPIOHANDLE_REQUEST_INPUT ) && - (lflags &GPIOHANDLE_REQUEST_OUTPUT )) -return -EINVAL ; - -/* -* Do not allow OPEN_SOURCE & OPEN_DRAIN flags in a single request. If -* the hardware actually supports enabling both at the same time the -* electrical result would be disastrous. -*/ -if ((lflags &GPIOHANDLE_REQUEST_OPEN_DRAIN ) && - (lflags &GPIOHANDLE_REQUEST_OPEN_SOURCE )) -return -EINVAL ; - -/* OPEN_DRAIN and OPEN_SOURCE flags only make sense for output mode. */ -if (!(lflags &GPIOHANDLE_REQUEST_OUTPUT ) && - ((lflags &GPIOHANDLE_REQUEST_OPEN_DRAIN ) || - (lflags &GPIOHANDLE_REQUEST_OPEN_SOURCE ))) -return -EINVAL ; - -/* Bias flags only allowed for input or output mode. */ -if (!((lflags &GPIOHANDLE_REQUEST_INPUT ) || - (lflags &GPIOHANDLE_REQUEST_OUTPUT )) && - ((lflags &GPIOHANDLE_REQUEST_BIAS_DISABLE ) || - (lflags &GPIOHANDLE_REQUEST_BIAS_PULL_UP ) || - (lflags &GPIOHANDLE_REQUEST_BIAS_PULL_DOWN ))) -return -EINVAL ; - -/* Only one bias flag can be set. */ -if (((lflags &GPIOHANDLE_REQUEST_BIAS_DISABLE ) && - (lflags & (GPIOHANDLE_REQUEST_BIAS_PULL_DOWN | -GPIOHANDLE_REQUEST_BIAS_PULL_UP ))) || - ((lflags &GPIOHANDLE_REQUEST_BIAS_PULL_DOWN ) && - (lflags &GPIOHANDLE_REQUEST_BIAS_PULL_UP ))) -return -EINVAL ; + ret =linehandle_validate_flags (lflags); +if (ret) +return ret; lh =kzalloc (sizeof (*lh),GFP_KERNEL );if (!lh)
Signature · 1 line
2.23.0
Kent Gibson
reviewerPATCH v2 2/2 gpio: add new SET_CONFIG ioctl() to gpio chardev
In reply to Kent GibsonAdd the GPIOHANDLE_SET_CONFIG_IOCTL to the gpio chardev. The ioctl allows some of the configuration of a requested handle to be changed without having to release the line.
The primary use case is the changing of direction for bi-directional lines.
Based on initial work by Bartosz Golaszewski <[email protected]>
--- drivers/gpio/gpiolib.c | 68 +++++++++++++++++++++++++++++++++++++++ include/uapi/linux/gpio.h | 18 +++++++++++ 2 files changed, 86 insertions(+)
drivers/gpio/gpiolib.c +69 -0
@@ -475,6 +475,73 @@ static int linehandle_validate_flags(u32 flags)return 0 ; } +static void linehandle_configure_flag (unsigned long *flagsp, +u32 bit,bool active) +{ +if (active) +set_bit (bit, flagsp); +else +clear_bit (bit, flagsp); +} + +static long linehandle_set_config (struct linehandle_state *lh, +void __user *ip) +{ +struct gpiohandle_config gcnf; +struct gpio_desc *desc; +int i, ret; +u32 lflags; +unsigned long *flagsp; + +if (copy_from_user (&gcnf, ip,sizeof (gcnf))) +return -EFAULT ; + + lflags = gcnf.flags; + ret =linehandle_validate_flags (lflags); +if (ret) +return ret; + +for (i =0 ; i < lh->numdescs; i++) { + desc = lh->descs[i]; + flagsp = &desc->flags; + +linehandle_configure_flag (flagsp,FLAG_ACTIVE_LOW , + lflags &GPIOHANDLE_REQUEST_ACTIVE_LOW ); + +linehandle_configure_flag (flagsp,FLAG_OPEN_DRAIN , + lflags &GPIOHANDLE_REQUEST_OPEN_DRAIN ); + +linehandle_configure_flag (flagsp,FLAG_OPEN_SOURCE , + lflags &GPIOHANDLE_REQUEST_OPEN_SOURCE ); + +linehandle_configure_flag (flagsp,FLAG_PULL_UP , + lflags &GPIOHANDLE_REQUEST_BIAS_PULL_UP ); + +linehandle_configure_flag (flagsp,FLAG_PULL_DOWN , + lflags &GPIOHANDLE_REQUEST_BIAS_PULL_DOWN ); + +linehandle_configure_flag (flagsp,FLAG_BIAS_DISABLE , + lflags &GPIOHANDLE_REQUEST_BIAS_DISABLE ); + +/* +* Lines have to be requested explicitly for input +* or output, else the line will be treated "as is". +*/ +if (lflags &GPIOHANDLE_REQUEST_OUTPUT ) { +int val = !!gcnf.default_values[i]; + + ret =gpiod_direction_output (desc, val); +if (ret) +return ret; + }else if (lflags &GPIOHANDLE_REQUEST_INPUT ) { + ret =gpiod_direction_input (desc); +if (ret) +return ret; + } + } +return 0 ; +} +static long linehandle_ioctl (struct file *filep,unsigned int cmd,unsigned long arg) {
@@ -525,6 +592,8 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd, lh->descs,NULL , vals); + }else if (cmd ==GPIOHANDLE_SET_CONFIG_IOCTL ) { +return linehandle_set_config (lh, ip); }return -EINVAL ; }
include/uapi/linux/gpio.h +18 -0
@@ -100,6 +100,24 @@ struct gpiohandle_request {int fd; }; +/** +* struct gpiohandle_config - Configuration for a GPIO handle request +* @flags: updated flags for the requested GPIO lines, such as +* GPIOHANDLE_REQUEST_OUTPUT, GPIOHANDLE_REQUEST_ACTIVE_LOW etc, OR:ed +* together +* @default_values: if the GPIOHANDLE_REQUEST_OUTPUT is set in flags, +* this specifies the default output value, should be 0 (low) or +* 1 (high), anything else than 0 or 1 will be interpreted as 1 (high) +* @padding: reserved for future use and should be zero filled +*/ +struct gpiohandle_config { +__u32 flags; +__u8 default_values[GPIOHANDLES_MAX ]; +__u32 padding[4 ];/* padding for future use */ +}; + +#define GPIOHANDLE_SET_CONFIG_IOCTL _IOWR(0xB4, 0x0a, struct gpiohandle_config) +/** * struct gpiohandle_data - Information of values on a GPIO handle * @values: when getting the state of lines this contains the current
Signature · 1 line
2.23.0
Quoted 4 lines · Kent Gibson
The patch series is against Bart's gpio/for-next branch[1] with v4 of my bias patch series "gpio: expose line bias flags to userspace" applied. This is necessary to support setting of the bias flags introduced there.
I'd like to apply this to test but I wanted to check should I still be applying all the patches from "[PATCH v4 0/5] gpio: expose line bias flags to userspace"?
thanks,
drew
Kent Gibson
reviewerQuoted 9 lines · Drew Fustini
Quoted 4 lines · Kent Gibson
The patch series is against Bart's gpio/for-next branch[1] with v4 of my bias patch series "gpio: expose line bias flags to userspace" applied. This is necessary to support setting of the bias flags introduced there.
I'd like to apply this to test but I wanted to check should I still be applying all the patches from "[PATCH v4 0/5] gpio: expose line bias flags to userspace"?
Yes, this patch series is based on top of the full bias patch series, so I would apply the whole series. It definitely requires patch 1 and 4 from that series, as they define some of the flags it uses.
I don't have any corresponding updates for libgpiod yet - and even my gpiod library only supports the new ioctl for testing.
I was holding off to see if the UAPI changes were ok and if Bart has any preference on additions to the libgpiod API, but I should look into that now.
Cheers,
Kent.