From d7c51b47ac11e66f547b55640405c1c474642d72 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Tue, 26 Apr 2016 10:35:29 +0200 Subject: gpio: userspace ABI for reading/writing GPIO lines This adds a userspace ABI for reading and writing GPIO lines. The mechanism returns an anonymous file handle to a request to read/write n offsets from a gpiochip. This file handle in turn accepts two ioctl()s: one that reads and one that writes values to the selected lines. - Handles can be requested as input/output, active low, open drain, open source, however when you issue a request for n lines with GPIO_GET_LINEHANDLE_IOCTL, they must all have the same flags, i.e. all inputs or all outputs, all open drain etc. If a granular control of the flags for each line is desired, they need to be requested individually, not in a batch. - The GPIOHANDLE_GET_LINE_VALUES_IOCTL read ioctl() can be issued also to output lines to verify that the hardware is in the expected state. - It reads and writes up to GPIOHANDLES_MAX lines at once, utilizing the .set_multiple() call in the driver if possible, making the call efficient if several lines can be written with a single register update. The limitation of GPIOHANDLES_MAX to 64 lines is done under the assumption that we may expect hardware that can issue a transaction updating 64 bits at an instant but unlikely anything larger than that. ChangeLog v2->v3: - Use gpiod_get_value_cansleep() so we support also slowpath GPIO drivers. - Fix up the UAPI docs kerneldoc. - Allocate the anonymous fd last, so that the release function don't get called until that point of something fails. After this point, skip the errorpath. ChangeLog v1->v2: - Handle ioctl_compat() properly based on a similar patch to the other ioctl() handling code. - Use _IOWR() as we pass pointers both in and out of the ioctl() - Use kmalloc() and kfree() for the linehandled, do not try to be fancy with devm_* it doesn't work the way I thought. - Fix const-correctness on the linehandle name field. Acked-by: Michael Welling Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 193 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 193 insertions(+) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 24f60d2..5f2e73e 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "gpiolib.h" @@ -310,6 +311,196 @@ static int gpiochip_set_desc_names(struct gpio_chip *gc) return 0; } +/* + * GPIO line handle management + */ + +/** + * struct linehandle_state - contains the state of a userspace handle + * @gdev: the GPIO device the handle pertains to + * @label: consumer label used to tag descriptors + * @descs: the GPIO descriptors held by this handle + * @numdescs: the number of descriptors held in the descs array + */ +struct linehandle_state { + struct gpio_device *gdev; + const char *label; + struct gpio_desc *descs[GPIOHANDLES_MAX]; + u32 numdescs; +}; + +static long linehandle_ioctl(struct file *filep, unsigned int cmd, + unsigned long arg) +{ + struct linehandle_state *lh = filep->private_data; + void __user *ip = (void __user *)arg; + struct gpiohandle_data ghd; + int i; + + if (cmd == GPIOHANDLE_GET_LINE_VALUES_IOCTL) { + int val; + + /* TODO: check if descriptors are really input */ + for (i = 0; i < lh->numdescs; i++) { + val = gpiod_get_value_cansleep(lh->descs[i]); + if (val < 0) + return val; + ghd.values[i] = val; + } + + if (copy_to_user(ip, &ghd, sizeof(ghd))) + return -EFAULT; + + return 0; + } else if (cmd == GPIOHANDLE_SET_LINE_VALUES_IOCTL) { + int vals[GPIOHANDLES_MAX]; + + /* TODO: check if descriptors are really output */ + if (copy_from_user(&ghd, ip, sizeof(ghd))) + return -EFAULT; + + /* Clamp all values to [0,1] */ + for (i = 0; i < lh->numdescs; i++) + vals[i] = !!ghd.values[i]; + + /* Reuse the array setting function */ + gpiod_set_array_value_complex(false, + true, + lh->numdescs, + lh->descs, + vals); + return 0; + } + return -EINVAL; +} + +#ifdef CONFIG_COMPAT +static long linehandle_ioctl_compat(struct file *filep, unsigned int cmd, + unsigned long arg) +{ + return linehandle_ioctl(filep, cmd, (unsigned long)compat_ptr(arg)); +} +#endif + +static int linehandle_release(struct inode *inode, struct file *filep) +{ + struct linehandle_state *lh = filep->private_data; + struct gpio_device *gdev = lh->gdev; + int i; + + for (i = 0; i < lh->numdescs; i++) + gpiod_free(lh->descs[i]); + kfree(lh->label); + kfree(lh); + put_device(&gdev->dev); + return 0; +} + +static const struct file_operations linehandle_fileops = { + .release = linehandle_release, + .owner = THIS_MODULE, + .llseek = noop_llseek, + .unlocked_ioctl = linehandle_ioctl, +#ifdef CONFIG_COMPAT + .compat_ioctl = linehandle_ioctl_compat, +#endif +}; + +static int linehandle_create(struct gpio_device *gdev, void __user *ip) +{ + struct gpiohandle_request handlereq; + struct linehandle_state *lh; + int fd, i, ret; + + if (copy_from_user(&handlereq, ip, sizeof(handlereq))) + return -EFAULT; + if ((handlereq.lines == 0) || (handlereq.lines > GPIOHANDLES_MAX)) + return -EINVAL; + + lh = kzalloc(sizeof(*lh), GFP_KERNEL); + if (!lh) + return -ENOMEM; + lh->gdev = gdev; + get_device(&gdev->dev); + + /* Make sure this is terminated */ + handlereq.consumer_label[sizeof(handlereq.consumer_label)-1] = '\0'; + if (strlen(handlereq.consumer_label)) { + lh->label = kstrdup(handlereq.consumer_label, + GFP_KERNEL); + if (!lh->label) { + ret = -ENOMEM; + goto out_free_lh; + } + } + + /* Request each GPIO */ + for (i = 0; i < handlereq.lines; i++) { + u32 offset = handlereq.lineoffsets[i]; + u32 lflags = handlereq.flags; + struct gpio_desc *desc; + + desc = &gdev->descs[offset]; + ret = gpiod_request(desc, lh->label); + if (ret) + goto out_free_descs; + lh->descs[i] = desc; + + if (lflags & GPIOHANDLE_REQUEST_ACTIVE_LOW) + set_bit(FLAG_ACTIVE_LOW, &desc->flags); + if (lflags & GPIOHANDLE_REQUEST_OPEN_DRAIN) + set_bit(FLAG_OPEN_DRAIN, &desc->flags); + if (lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE) + set_bit(FLAG_OPEN_SOURCE, &desc->flags); + + /* + * 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 = !!handlereq.default_values[i]; + + ret = gpiod_direction_output(desc, val); + if (ret) + goto out_free_descs; + } else if (lflags & GPIOHANDLE_REQUEST_INPUT) { + ret = gpiod_direction_input(desc); + if (ret) + goto out_free_descs; + } + dev_dbg(&gdev->dev, "registered chardev handle for line %d\n", + offset); + } + lh->numdescs = handlereq.lines; + + fd = anon_inode_getfd("gpio-linehandle", + &linehandle_fileops, + lh, + O_RDONLY | O_CLOEXEC); + if (fd < 0) { + ret = fd; + goto out_free_descs; + } + + handlereq.fd = fd; + if (copy_to_user(ip, &handlereq, sizeof(handlereq))) + return -EFAULT; + + dev_dbg(&gdev->dev, "registered chardev handle for %d lines\n", + lh->numdescs); + + return 0; + +out_free_descs: + for (; i >= 0; i--) + gpiod_free(lh->descs[i]); + kfree(lh->label); +out_free_lh: + kfree(lh); + put_device(&gdev->dev); + return ret; +} + /** * gpio_ioctl() - ioctl handler for the GPIO chardev */ @@ -385,6 +576,8 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) if (copy_to_user(ip, &lineinfo, sizeof(lineinfo))) return -EFAULT; return 0; + } else if (cmd == GPIO_GET_LINEHANDLE_IOCTL) { + return linehandle_create(gdev, ip); } return -EINVAL; } -- cgit v1.1 From 61f922db72216b00386581c851db9c9095961522 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Thu, 2 Jun 2016 11:30:15 +0200 Subject: gpio: userspace ABI for reading GPIO line events This adds an ABI for listening to events on GPIO lines. The mechanism returns an anonymous file handle to a request to listen to a specific offset on a specific gpiochip. To fetch the stream of events from the file handle, userspace simply reads an event. - Events can be requested with the same flags as ordinary handles, i.e. open drain or open source. An ioctl() call GPIO_GET_LINEEVENT_IOCTL is issued indicating the desired line. - Events can be requested for falling edge events, rising edge events, or both. - All events are timestamped using the kernel real time nanosecond timestamp (the same as is used by IIO). - The supplied consumer label will appear in "lsgpio" listings of the lines, and in /proc/interrupts as the mechanism will request an interrupt from the gpio chip. - Events are not supported on gpiochips that do not serve interrupts (no legal .to_irq() call). The event interrupt is threaded to avoid any realtime problems. - It is possible to also directly read the current value of the registered GPIO line by issuing the same GPIOHANDLE_GET_LINE_VALUES_IOCTL as used by the line handles. Setting the value is not supported: we do not listen to events on output lines. This ABI is strongly influenced by Industrial I/O and surpasses the old sysfs ABI by providing proper precision timestamps, making it possible to set flags like open drain, and put consumer names on the GPIO lines. Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 298 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 298 insertions(+) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 5f2e73e..5239230 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -22,6 +22,9 @@ #include #include #include +#include +#include +#include #include #include "gpiolib.h" @@ -501,6 +504,299 @@ out_free_lh: return ret; } +/* + * GPIO line event management + */ + +/** + * struct lineevent_state - contains the state of a userspace event + * @gdev: the GPIO device the event pertains to + * @label: consumer label used to tag descriptors + * @desc: the GPIO descriptor held by this event + * @eflags: the event flags this line was requested with + * @irq: the interrupt that trigger in response to events on this GPIO + * @wait: wait queue that handles blocking reads of events + * @events: KFIFO for the GPIO events + * @read_lock: mutex lock to protect reads from colliding with adding + * new events to the FIFO + */ +struct lineevent_state { + struct gpio_device *gdev; + const char *label; + struct gpio_desc *desc; + u32 eflags; + int irq; + wait_queue_head_t wait; + DECLARE_KFIFO(events, struct gpioevent_data, 16); + struct mutex read_lock; +}; + +static unsigned int lineevent_poll(struct file *filep, + struct poll_table_struct *wait) +{ + struct lineevent_state *le = filep->private_data; + unsigned int events = 0; + + poll_wait(filep, &le->wait, wait); + + if (!kfifo_is_empty(&le->events)) + events = POLLIN | POLLRDNORM; + + return events; +} + + +static ssize_t lineevent_read(struct file *filep, + char __user *buf, + size_t count, + loff_t *f_ps) +{ + struct lineevent_state *le = filep->private_data; + unsigned int copied; + int ret; + + if (count < sizeof(struct gpioevent_data)) + return -EINVAL; + + do { + if (kfifo_is_empty(&le->events)) { + if (filep->f_flags & O_NONBLOCK) + return -EAGAIN; + + ret = wait_event_interruptible(le->wait, + !kfifo_is_empty(&le->events)); + if (ret) + return ret; + } + + if (mutex_lock_interruptible(&le->read_lock)) + return -ERESTARTSYS; + ret = kfifo_to_user(&le->events, buf, count, &copied); + mutex_unlock(&le->read_lock); + + if (ret) + return ret; + + /* + * If we couldn't read anything from the fifo (a different + * thread might have been faster) we either return -EAGAIN if + * the file descriptor is non-blocking, otherwise we go back to + * sleep and wait for more data to arrive. + */ + if (copied == 0 && (filep->f_flags & O_NONBLOCK)) + return -EAGAIN; + + } while (copied == 0); + + return copied; +} + +static int lineevent_release(struct inode *inode, struct file *filep) +{ + struct lineevent_state *le = filep->private_data; + struct gpio_device *gdev = le->gdev; + + free_irq(le->irq, le); + gpiod_free(le->desc); + kfree(le->label); + kfree(le); + put_device(&gdev->dev); + return 0; +} + +static long lineevent_ioctl(struct file *filep, unsigned int cmd, + unsigned long arg) +{ + struct lineevent_state *le = filep->private_data; + void __user *ip = (void __user *)arg; + struct gpiohandle_data ghd; + + /* + * We can get the value for an event line but not set it, + * because it is input by definition. + */ + if (cmd == GPIOHANDLE_GET_LINE_VALUES_IOCTL) { + int val; + + val = gpiod_get_value_cansleep(le->desc); + if (val < 0) + return val; + ghd.values[0] = val; + + if (copy_to_user(ip, &ghd, sizeof(ghd))) + return -EFAULT; + + return 0; + } + return -EINVAL; +} + +#ifdef CONFIG_COMPAT +static long lineevent_ioctl_compat(struct file *filep, unsigned int cmd, + unsigned long arg) +{ + return lineevent_ioctl(filep, cmd, (unsigned long)compat_ptr(arg)); +} +#endif + +static const struct file_operations lineevent_fileops = { + .release = lineevent_release, + .read = lineevent_read, + .poll = lineevent_poll, + .owner = THIS_MODULE, + .llseek = noop_llseek, + .unlocked_ioctl = lineevent_ioctl, +#ifdef CONFIG_COMPAT + .compat_ioctl = lineevent_ioctl_compat, +#endif +}; + +irqreturn_t lineevent_irq_thread(int irq, void *p) +{ + struct lineevent_state *le = p; + struct gpioevent_data ge; + int ret; + + ge.timestamp = ktime_get_real_ns(); + + if (le->eflags & GPIOEVENT_REQUEST_BOTH_EDGES) { + int level = gpiod_get_value_cansleep(le->desc); + + if (level) + /* Emit low-to-high event */ + ge.id = GPIOEVENT_EVENT_RISING_EDGE; + else + /* Emit high-to-low event */ + ge.id = GPIOEVENT_EVENT_FALLING_EDGE; + } else if (le->eflags & GPIOEVENT_REQUEST_RISING_EDGE) { + /* Emit low-to-high event */ + ge.id = GPIOEVENT_EVENT_RISING_EDGE; + } else if (le->eflags & GPIOEVENT_REQUEST_FALLING_EDGE) { + /* Emit high-to-low event */ + ge.id = GPIOEVENT_EVENT_FALLING_EDGE; + } + + ret = kfifo_put(&le->events, ge); + if (ret != 0) + wake_up_poll(&le->wait, POLLIN); + + return IRQ_HANDLED; +} + +static int lineevent_create(struct gpio_device *gdev, void __user *ip) +{ + struct gpioevent_request eventreq; + struct lineevent_state *le; + struct gpio_desc *desc; + u32 offset; + u32 lflags; + u32 eflags; + int fd; + int ret; + int irqflags = 0; + + if (copy_from_user(&eventreq, ip, sizeof(eventreq))) + return -EFAULT; + + le = kzalloc(sizeof(*le), GFP_KERNEL); + if (!le) + return -ENOMEM; + le->gdev = gdev; + get_device(&gdev->dev); + + /* Make sure this is terminated */ + eventreq.consumer_label[sizeof(eventreq.consumer_label)-1] = '\0'; + if (strlen(eventreq.consumer_label)) { + le->label = kstrdup(eventreq.consumer_label, + GFP_KERNEL); + if (!le->label) { + ret = -ENOMEM; + goto out_free_le; + } + } + + offset = eventreq.lineoffset; + lflags = eventreq.handleflags; + eflags = eventreq.eventflags; + + /* This is just wrong: we don't look for events on output lines */ + if (lflags & GPIOHANDLE_REQUEST_OUTPUT) { + ret = -EINVAL; + goto out_free_label; + } + + desc = &gdev->descs[offset]; + ret = gpiod_request(desc, le->label); + if (ret) + goto out_free_desc; + le->desc = desc; + le->eflags = eflags; + + if (lflags & GPIOHANDLE_REQUEST_ACTIVE_LOW) + set_bit(FLAG_ACTIVE_LOW, &desc->flags); + if (lflags & GPIOHANDLE_REQUEST_OPEN_DRAIN) + set_bit(FLAG_OPEN_DRAIN, &desc->flags); + if (lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE) + set_bit(FLAG_OPEN_SOURCE, &desc->flags); + + ret = gpiod_direction_input(desc); + if (ret) + goto out_free_desc; + + le->irq = gpiod_to_irq(desc); + if (le->irq <= 0) { + ret = -ENODEV; + goto out_free_desc; + } + + if (eflags & GPIOEVENT_REQUEST_RISING_EDGE) + irqflags |= IRQF_TRIGGER_RISING; + if (eflags & GPIOEVENT_REQUEST_FALLING_EDGE) + irqflags |= IRQF_TRIGGER_FALLING; + irqflags |= IRQF_ONESHOT; + irqflags |= IRQF_SHARED; + + INIT_KFIFO(le->events); + init_waitqueue_head(&le->wait); + mutex_init(&le->read_lock); + + /* Request a thread to read the events */ + ret = request_threaded_irq(le->irq, + NULL, + lineevent_irq_thread, + irqflags, + le->label, + le); + if (ret) + goto out_free_desc; + + fd = anon_inode_getfd("gpio-event", + &lineevent_fileops, + le, + O_RDONLY | O_CLOEXEC); + if (fd < 0) { + ret = fd; + goto out_free_irq; + } + + eventreq.fd = fd; + if (copy_to_user(ip, &eventreq, sizeof(eventreq))) + return -EFAULT; + + return 0; + +out_free_irq: + free_irq(le->irq, le); +out_free_desc: + gpiod_free(le->desc); +out_free_label: + kfree(le->label); +out_free_le: + kfree(le); + put_device(&gdev->dev); + return ret; +} + /** * gpio_ioctl() - ioctl handler for the GPIO chardev */ @@ -578,6 +874,8 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) return 0; } else if (cmd == GPIO_GET_LINEHANDLE_IOCTL) { return linehandle_create(gdev, ip); + } else if (cmd == GPIO_GET_LINEEVENT_IOCTL) { + return lineevent_create(gdev, ip); } return -EINVAL; } -- cgit v1.1 From bc0207a5461169eba13e9421bd7632399b72e3ab Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Thu, 16 Jun 2016 11:02:41 +0200 Subject: gpiolib: avoid uninitialized data in gpio kfifo gcc reports a theoretical case for returning uninitialized data in the kfifo when a GPIO interrupt happens and neither GPIOEVENT_REQUEST_RISING_EDGE nor GPIOEVENT_REQUEST_FALLING_EDGE are set: drivers/gpio/gpiolib.c: In function 'lineevent_irq_thread': drivers/gpio/gpiolib.c:683:87: error: 'ge.id' may be used uninitialized in this function [-Werror=maybe-uninitialized] This case should not happen, but to be on the safe side, let's return from the irq handler without adding data to the FIFO to ensure we can never leak stack data to user space. Signed-off-by: Arnd Bergmann Fixes: 61f922db7221 ("gpio: userspace ABI for reading GPIO line events") Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 5239230..c826844 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -674,6 +674,8 @@ irqreturn_t lineevent_irq_thread(int irq, void *p) } else if (le->eflags & GPIOEVENT_REQUEST_FALLING_EDGE) { /* Emit high-to-low event */ ge.id = GPIOEVENT_EVENT_FALLING_EDGE; + } else { + return IRQ_NONE; } ret = kfifo_put(&le->events, ge); -- cgit v1.1 From e2f608be640a126da50be605e1a81b988c9ac0d6 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Sat, 18 Jun 2016 10:56:43 +0200 Subject: gpio: make the iterator point to last handle When initializing the GPIO handles, we use the iterator (i) to back off if something goes wrong. But since the iterator is also used after we pass the loop, we must decrement by one after exiting the loop so that we point at the last element in the array. Reported-by: Dan Carpenter Cc: Dan Carpenter Cc: Walter Harms Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index c826844..b504364 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -474,6 +474,8 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip) dev_dbg(&gdev->dev, "registered chardev handle for line %d\n", offset); } + /* Let i point at the last handle */ + i--; lh->numdescs = handlereq.lines; fd = anon_inode_getfd("gpio-linehandle", -- cgit v1.1 From 33265b17e06e2d84900efebfa8620d2f5bfcc5de Mon Sep 17 00:00:00 2001 From: Ben Dooks Date: Fri, 17 Jun 2016 16:03:13 +0100 Subject: gpiolib: make lineevent_irq_thread static The lineevent_irq_thread is not exported, so make it static to fix the following warning: drivers/gpio/gpiolib.c:654:13: warning: symbol 'lineevent_irq_thread' was not declared. Should it be static? Signed-off-by: Ben Dooks Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index b504364..5a21a6a 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -653,7 +653,7 @@ static const struct file_operations lineevent_fileops = { #endif }; -irqreturn_t lineevent_irq_thread(int irq, void *p) +static irqreturn_t lineevent_irq_thread(int irq, void *p) { struct lineevent_state *le = p; struct gpioevent_data ge; -- cgit v1.1 From 7e7c059cb50c7c72d5a393b2c34fc57de1b01b55 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Wed, 22 Jun 2016 16:31:54 +0200 Subject: gpio: convince line to become input in irq helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generic IRQ helper library just checks if the IRQ line is set as input before activating it for interrupts. As we recently started to check things better with .get_dir() it turns out that it's good to try to convince the line to become an input before attempting to lock it as IRQ. Reviewed-by: Björn Andersson Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 5a21a6a..b195ec4 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1505,6 +1505,25 @@ static int gpiochip_irq_reqres(struct irq_data *d) if (!try_module_get(chip->gpiodev->owner)) return -ENODEV; + /* + * If it is possible to switch this GPIO to an input + * this is a good time to do it. + */ + if (chip->direction_input) { + struct gpio_desc *desc; + int ret; + + desc = gpiochip_get_desc(chip, d->hwirq); + if (IS_ERR(desc)) + return PTR_ERR(desc); + + ret = chip->direction_input(chip, d->hwirq); + if (ret) + return ret; + + clear_bit(FLAG_IS_OUT, &desc->flags); + } + if (gpiochip_lock_as_irq(chip, d->hwirq)) { chip_err(chip, "unable to lock HW IRQ %lu for IRQ\n", -- cgit v1.1 From d932cd49182f97966d196fb5301bfca90f58a360 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Mon, 4 Jul 2016 13:13:04 +0200 Subject: gpio: free handles in fringe cases If we fail when copying the ioctl() struct to userspace we still need to clean up the cruft otherwise left behind or it will stay around until the issuing process terminates the file handle. Reported-by: Alexander Stein Acked-by: Alexander Stein Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index b195ec4..69efe27 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -488,8 +488,10 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip) } handlereq.fd = fd; - if (copy_to_user(ip, &handlereq, sizeof(handlereq))) - return -EFAULT; + if (copy_to_user(ip, &handlereq, sizeof(handlereq))) { + ret = -EFAULT; + goto out_free_descs; + } dev_dbg(&gdev->dev, "registered chardev handle for %d lines\n", lh->numdescs); @@ -784,8 +786,10 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip) } eventreq.fd = fd; - if (copy_to_user(ip, &eventreq, sizeof(eventreq))) - return -EFAULT; + if (copy_to_user(ip, &eventreq, sizeof(eventreq))) { + ret = -EFAULT; + goto out_free_irq; + } return 0; -- cgit v1.1 From acc6e331b62275570d23b20ced6296812023967f Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Tue, 5 Jul 2016 14:11:14 +0200 Subject: gpio: of: Allow overriding the device node When registering a GPIO chip, drivers can override the device tree node associated with the chip by setting the chip's ->of_node field. If set, this field is supposed to take precedence over the ->parent->of_node field, but the code doesn't actually do that. Commit 762c2e46c059 ("gpio: of: remove of_gpiochip_and_xlate() and struct gg_data") exposes this because it now no longer matches on the GPIO chip's ->of_node field, but the GPIO device's ->of_node field that is set using the procedure described above. Signed-off-by: Thierry Reding Acked-by: Alexandre Courbot Reviewed-by: Masahiro Yamada Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 69efe27..6dc3917 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1049,13 +1049,14 @@ int gpiochip_add_data(struct gpio_chip *chip, void *data) if (chip->parent) { gdev->dev.parent = chip->parent; gdev->dev.of_node = chip->parent->of_node; - } else { + } + #ifdef CONFIG_OF_GPIO /* If the gpiochip has an assigned OF node this takes precedence */ - if (chip->of_node) - gdev->dev.of_node = chip->of_node; + if (chip->of_node) + gdev->dev.of_node = chip->of_node; #endif - } + gdev->id = ida_simple_get(&gpio_ida, 0, 0, GFP_KERNEL); if (gdev->id < 0) { status = gdev->id; -- cgit v1.1 From da17f8a11378917a97019be33fed35893b7b7e1a Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Fri, 1 Jul 2016 17:40:09 +0200 Subject: gpiolib: of_find_gpio(): Don't discard errors Since commit dd34c37aa3e8 ("gpio: of: Allow -gpio suffix for property names") when requesting a GPIO from the devicetree gpiolib looks for properties with both the '-gpio' and the '-gpios' suffix. This was implemented by first searching for the property with the '-gpios' suffix and if that yields an error try the '-gpio' suffix. This approach has the issue that any error returned when looking for the '-gpios' suffix is silently discarded. Commit 06fc3b70f1dc ("gpio: of: Fix handling for deferred probe for -gpio suffix") partially addressed the issue by treating the EPROBE_DEFER error as a special condition. This fixed the case when the property is specified, but the GPIO provider is not ready yet. But there are other cases in which of_get_named_gpiod_flags() returns an error even though the property is specified, e.g. if the specification is incorrect. of_find_gpio() should only try to look for the property with the '-gpio' suffix if no property with the '-gpios' suffix was found. If the property was not found of_get_named_gpiod_flags() will return -ENOENT, so update the condition to abort and propagate the error to the caller in all other cases. This is important for gpiod_get_optinal() and friends to behave correctly in case the specifier contains errors. Without this patch they'll return NULL if the property uses the '-gpios' suffix and the specifier contains errors, which falsely indicates to the caller that no GPIO was specified. Signed-off-by: Lars-Peter Clausen Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 6dc3917..5b0f454 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -2845,7 +2845,7 @@ static struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, desc = of_get_named_gpiod_flags(dev->of_node, prop_name, idx, &of_flags); - if (!IS_ERR(desc) || (PTR_ERR(desc) == -EPROBE_DEFER)) + if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT)) break; } -- cgit v1.1 From 78456d6ff815894e593675fc524cade9844501d5 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Wed, 6 Jul 2016 14:40:08 +0200 Subject: Revert "gpio: convince line to become input in irq helper" This reverts commit 7e7c059cb50c7c72d5a393b2c34fc57de1b01b55. I was wrong about trying to do this, as it breaks the orthogonality between gpiochips and irqchips. Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 19 ------------------- 1 file changed, 19 deletions(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 5b0f454..2dff169 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1510,25 +1510,6 @@ static int gpiochip_irq_reqres(struct irq_data *d) if (!try_module_get(chip->gpiodev->owner)) return -ENODEV; - /* - * If it is possible to switch this GPIO to an input - * this is a good time to do it. - */ - if (chip->direction_input) { - struct gpio_desc *desc; - int ret; - - desc = gpiochip_get_desc(chip, d->hwirq); - if (IS_ERR(desc)) - return PTR_ERR(desc); - - ret = chip->direction_input(chip, d->hwirq); - if (ret) - return ret; - - clear_bit(FLAG_IS_OUT, &desc->flags); - } - if (gpiochip_lock_as_irq(chip, d->hwirq)) { chip_err(chip, "unable to lock HW IRQ %lu for IRQ\n", -- cgit v1.1 From ee4fc4013e0d62a3669101299dede0216bc6873e Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Wed, 6 Jul 2016 12:26:52 +0000 Subject: gpiolib: remove duplicated include from gpiolib.c Remove duplicated include. Signed-off-by: Wei Yongjun Signed-off-by: Linus Walleij --- drivers/gpio/gpiolib.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/gpio/gpiolib.c') diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 2dff169..d2ba286 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -16,7 +16,6 @@ #include #include #include -#include #include #include #include -- cgit v1.1