From a630fe34ddc09ec9b82df4867a8680d0929fe926 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 28 Jan 2020 22:08:45 +0100 Subject: [PATCH 01/10] gpio: pxa: Avoid a warning when gpio0 and gpio1 IRQS are not there Not all platforms use those. Let's use platform_get_irq_byname_optional() instead platform_get_irq_byname() so that we avoid a useless warning: [ 1.359455] pxa-gpio d4019000.gpio: IRQ gpio0 not found [ 1.359583] pxa-gpio d4019000.gpio: IRQ gpio1 not found Signed-off-by: Lubomir Rintel Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-pxa.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/gpio-pxa.c b/drivers/gpio/gpio-pxa.c index 9888b62f37af..567742d962ae 100644 --- a/drivers/gpio/gpio-pxa.c +++ b/drivers/gpio/gpio-pxa.c @@ -652,8 +652,8 @@ static int pxa_gpio_probe(struct platform_device *pdev) if (!pchip->irqdomain) return -ENOMEM; - irq0 = platform_get_irq_byname(pdev, "gpio0"); - irq1 = platform_get_irq_byname(pdev, "gpio1"); + irq0 = platform_get_irq_byname_optional(pdev, "gpio0"); + irq1 = platform_get_irq_byname_optional(pdev, "gpio1"); irq_mux = platform_get_irq_byname(pdev, "gpio_mux"); if ((irq0 > 0 && irq1 <= 0) || (irq0 <= 0 && irq1 > 0) || (irq_mux <= 0)) From 47d7d116661993499d626f7ec6f7679e83d59f15 Mon Sep 17 00:00:00 2001 From: Axel Lin Date: Fri, 31 Jan 2020 20:29:17 +0800 Subject: [PATCH 02/10] gpio: wcd934x: Don't change gpio direction in wcd_gpio_set The .set callback should just set output value. Signed-off-by: Axel Lin Reviewed-by: Srinivas Kandagatla Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-wcd934x.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-wcd934x.c b/drivers/gpio/gpio-wcd934x.c index 74913f2e5697..9d4ec8941b9b 100644 --- a/drivers/gpio/gpio-wcd934x.c +++ b/drivers/gpio/gpio-wcd934x.c @@ -66,7 +66,10 @@ static int wcd_gpio_get(struct gpio_chip *chip, unsigned int pin) static void wcd_gpio_set(struct gpio_chip *chip, unsigned int pin, int val) { - wcd_gpio_direction_output(chip, pin, val); + struct wcd_gpio_data *data = gpiochip_get_data(chip); + + regmap_update_bits(data->map, WCD_REG_VAL_CTL_OFFSET, + WCD_PIN_MASK(pin), val ? WCD_PIN_MASK(pin) : 0); } static int wcd_gpio_probe(struct platform_device *pdev) From 47203198ed3df0f6896d07613182c05cb94110a5 Mon Sep 17 00:00:00 2001 From: Axel Lin Date: Fri, 31 Jan 2020 20:29:18 +0800 Subject: [PATCH 03/10] gpio: wcd934x: Fix logic of wcd_gpio_get The check with register value and mask should be & rather than &&. While at it, also use "unsigned int" for value variable because regmap_read() takes unsigned int *val argument. Signed-off-by: Axel Lin Reviewed-by: Srinivas Kandagatla Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpio-wcd934x.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpio/gpio-wcd934x.c b/drivers/gpio/gpio-wcd934x.c index 9d4ec8941b9b..1cbce5990855 100644 --- a/drivers/gpio/gpio-wcd934x.c +++ b/drivers/gpio/gpio-wcd934x.c @@ -57,11 +57,11 @@ static int wcd_gpio_direction_output(struct gpio_chip *chip, unsigned int pin, static int wcd_gpio_get(struct gpio_chip *chip, unsigned int pin) { struct wcd_gpio_data *data = gpiochip_get_data(chip); - int value; + unsigned int value; regmap_read(data->map, WCD_REG_VAL_CTL_OFFSET, &value); - return !!(value && WCD_PIN_MASK(pin)); + return !!(value & WCD_PIN_MASK(pin)); } static void wcd_gpio_set(struct gpio_chip *chip, unsigned int pin, int val) From 3f2e4c11e136e2cffd60dbc840b59ff65f017328 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Tue, 17 Dec 2019 18:48:55 +0100 Subject: [PATCH 04/10] kfifo: provide noirqsave variants of spinlocked in and out helpers Provide variants of spinlocked kfifo_in() and kfifo_out() routines which don't disable interrupts. Signed-off-by: Bartosz Golaszewski Acked-by: Stefani Seibold --- include/linux/kfifo.h | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/include/linux/kfifo.h b/include/linux/kfifo.h index fc4b0b10210f..123c200ed7cb 100644 --- a/include/linux/kfifo.h +++ b/include/linux/kfifo.h @@ -517,6 +517,26 @@ __kfifo_uint_must_check_helper( \ __ret; \ }) +/** + * kfifo_in_spinlocked_noirqsave - put data into fifo using a spinlock for + * locking, don't disable interrupts + * @fifo: address of the fifo to be used + * @buf: the data to be added + * @n: number of elements to be added + * @lock: pointer to the spinlock to use for locking + * + * This is a variant of kfifo_in_spinlocked() but uses spin_lock/unlock() + * for locking and doesn't disable interrupts. + */ +#define kfifo_in_spinlocked_noirqsave(fifo, buf, n, lock) \ +({ \ + unsigned int __ret; \ + spin_lock(lock); \ + __ret = kfifo_in(fifo, buf, n); \ + spin_unlock(lock); \ + __ret; \ +}) + /* alias for kfifo_in_spinlocked, will be removed in a future release */ #define kfifo_in_locked(fifo, buf, n, lock) \ kfifo_in_spinlocked(fifo, buf, n, lock) @@ -569,6 +589,28 @@ __kfifo_uint_must_check_helper( \ }) \ ) +/** + * kfifo_out_spinlocked_noirqsave - get data from the fifo using a spinlock + * for locking, don't disable interrupts + * @fifo: address of the fifo to be used + * @buf: pointer to the storage buffer + * @n: max. number of elements to get + * @lock: pointer to the spinlock to use for locking + * + * This is a variant of kfifo_out_spinlocked() which uses spin_lock/unlock() + * for locking and doesn't disable interrupts. + */ +#define kfifo_out_spinlocked_noirqsave(fifo, buf, n, lock) \ +__kfifo_uint_must_check_helper( \ +({ \ + unsigned int __ret; \ + spin_lock(lock); \ + __ret = kfifo_out(fifo, buf, n); \ + spin_unlock(lock); \ + __ret; \ +}) \ +) + /* alias for kfifo_out_spinlocked, will be removed in a future release */ #define kfifo_out_locked(fifo, buf, n, lock) \ kfifo_out_spinlocked(fifo, buf, n, lock) From 5195a89e8583bba43ec13871a7226763e401b44e Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Tue, 17 Dec 2019 11:30:59 +0100 Subject: [PATCH 05/10] kfifo: provide kfifo_is_empty_spinlocked() Provide two spinlocked versions of kfifo_is_empty() to be used with spinlocked variants of kfifo_in() and kfifo_out(). Signed-off-by: Bartosz Golaszewski Acked-by: Stefani Seibold --- include/linux/kfifo.h | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/include/linux/kfifo.h b/include/linux/kfifo.h index 123c200ed7cb..86249476b57f 100644 --- a/include/linux/kfifo.h +++ b/include/linux/kfifo.h @@ -246,6 +246,37 @@ __kfifo_int_must_check_helper(int val) __tmpq->kfifo.in == __tmpq->kfifo.out; \ }) +/** + * kfifo_is_empty_spinlocked - returns true if the fifo is empty using + * a spinlock for locking + * @fifo: address of the fifo to be used + * @lock: spinlock to be used for locking + */ +#define kfifo_is_empty_spinlocked(fifo, lock) \ +({ \ + unsigned long __flags; \ + bool __ret; \ + spin_lock_irqsave(lock, __flags); \ + __ret = kfifo_is_empty(fifo); \ + spin_unlock_irqrestore(lock, __flags); \ + __ret; \ +}) + +/** + * kfifo_is_empty_spinlocked_noirqsave - returns true if the fifo is empty + * using a spinlock for locking, doesn't disable interrupts + * @fifo: address of the fifo to be used + * @lock: spinlock to be used for locking + */ +#define kfifo_is_empty_spinlocked_noirqsave(fifo, lock) \ +({ \ + bool __ret; \ + spin_lock(lock); \ + __ret = kfifo_is_empty(fifo); \ + spin_unlock(lock); \ + __ret; \ +}) + /** * kfifo_is_full - returns true if the fifo is full * @fifo: address of the fifo to be used From dea9c80ee6726986d90260f135c83545427cbc4e Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 27 Nov 2019 12:19:21 +0100 Subject: [PATCH 06/10] gpiolib: rework the locking mechanism for lineevent kfifo The read_lock mutex is supposed to prevent collisions between reading and writing to the line event kfifo but it's actually only taken when the events are being read from it. Drop the mutex entirely and reuse the spinlock made available to us in the waitqueue struct. Take the lock whenever the fifo is modified or inspected. Drop the call to kfifo_to_user() and instead first extract the new element from kfifo when the lock is taken and only then pass it on to the user after the spinlock is released. Signed-off-by: Bartosz Golaszewski Reviewed-by: Andy Shevchenko --- drivers/gpio/gpiolib.c | 66 +++++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 753283486037..43d98309e725 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -787,8 +787,6 @@ out_free_lh: * @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 * @timestamp: cache for the timestamp storing it between hardirq * and IRQ thread, used to bring the timestamp close to the actual * event @@ -801,7 +799,6 @@ struct lineevent_state { int irq; wait_queue_head_t wait; DECLARE_KFIFO(events, struct gpioevent_data, 16); - struct mutex read_lock; u64 timestamp; }; @@ -817,7 +814,7 @@ static __poll_t lineevent_poll(struct file *filep, poll_wait(filep, &le->wait, wait); - if (!kfifo_is_empty(&le->events)) + if (!kfifo_is_empty_spinlocked_noirqsave(&le->events, &le->wait.lock)) events = EPOLLIN | EPOLLRDNORM; return events; @@ -830,43 +827,52 @@ static ssize_t lineevent_read(struct file *filep, loff_t *f_ps) { struct lineevent_state *le = filep->private_data; - unsigned int copied; + struct gpioevent_data event; + ssize_t bytes_read = 0; int ret; - if (count < sizeof(struct gpioevent_data)) + if (count < sizeof(event)) return -EINVAL; do { + spin_lock(&le->wait.lock); if (kfifo_is_empty(&le->events)) { - if (filep->f_flags & O_NONBLOCK) - return -EAGAIN; + if (bytes_read) { + spin_unlock(&le->wait.lock); + return bytes_read; + } - ret = wait_event_interruptible(le->wait, + if (filep->f_flags & O_NONBLOCK) { + spin_unlock(&le->wait.lock); + return -EAGAIN; + } + + ret = wait_event_interruptible_locked(le->wait, !kfifo_is_empty(&le->events)); - if (ret) + if (ret) { + spin_unlock(&le->wait.lock); 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); + ret = kfifo_out(&le->events, &event, 1); + spin_unlock(&le->wait.lock); + if (ret != 1) { + /* + * This should never happen - we were holding the lock + * from the moment we learned the fifo is no longer + * empty until now. + */ + ret = -EIO; + break; + } - if (ret) - return ret; + if (copy_to_user(buf + bytes_read, &event, sizeof(event))) + return -EFAULT; + bytes_read += sizeof(event); + } while (count >= bytes_read + sizeof(event)); - /* - * 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; + return bytes_read; } static int lineevent_release(struct inode *inode, struct file *filep) @@ -968,7 +974,8 @@ static irqreturn_t lineevent_irq_thread(int irq, void *p) return IRQ_NONE; } - ret = kfifo_put(&le->events, ge); + ret = kfifo_in_spinlocked_noirqsave(&le->events, &ge, + 1, &le->wait.lock); if (ret) wake_up_poll(&le->wait, EPOLLIN); @@ -1083,7 +1090,6 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip) 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, From 248ae1752e91438cb7cfc8432b88bc34410e8e4e Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Fri, 29 Nov 2019 11:22:18 +0100 Subject: [PATCH 07/10] gpiolib: emit a debug message when adding events to a full kfifo Currently if the line-event kfifo is full, we just silently drop any new events. Add a ratelimited debug message so that we at least have some trace in the kernel log of event overflow. Signed-off-by: Bartosz Golaszewski --- drivers/gpio/gpiolib.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 43d98309e725..36afe0b2b150 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -978,6 +978,8 @@ static irqreturn_t lineevent_irq_thread(int irq, void *p) 1, &le->wait.lock); if (ret) wake_up_poll(&le->wait, EPOLLIN); + else + pr_debug_ratelimited("event FIFO is full - event dropped\n"); return IRQ_HANDLED; } From d2ac25798208fb85f866056cd7d8030eb919da99 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Tue, 3 Dec 2019 14:08:44 +0100 Subject: [PATCH 08/10] gpiolib: provide a dedicated function for setting lineinfo We'll soon be filling out the gpioline_info structure in multiple places. Add a separate function that given a gpio_desc sets all relevant fields. Signed-off-by: Bartosz Golaszewski Reviewed-by: Andy Shevchenko --- drivers/gpio/gpiolib.c | 98 ++++++++++++++++++++++++------------------ 1 file changed, 55 insertions(+), 43 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 36afe0b2b150..443321f9cf63 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1147,6 +1147,60 @@ out_free_le: return ret; } +static void gpio_desc_to_lineinfo(struct gpio_desc *desc, + struct gpioline_info *info) +{ + struct gpio_chip *chip = desc->gdev->chip; + unsigned long flags; + + spin_lock_irqsave(&gpio_lock, flags); + + if (desc->name) { + strncpy(info->name, desc->name, sizeof(info->name)); + info->name[sizeof(info->name) - 1] = '\0'; + } else { + info->name[0] = '\0'; + } + + if (desc->label) { + strncpy(info->consumer, desc->label, sizeof(info->consumer)); + info->consumer[sizeof(info->consumer) - 1] = '\0'; + } else { + info->consumer[0] = '\0'; + } + + /* + * Userspace only need to know that the kernel is using this GPIO so + * it can't use it. + */ + info->flags = 0; + if (test_bit(FLAG_REQUESTED, &desc->flags) || + test_bit(FLAG_IS_HOGGED, &desc->flags) || + test_bit(FLAG_USED_AS_IRQ, &desc->flags) || + test_bit(FLAG_EXPORT, &desc->flags) || + test_bit(FLAG_SYSFS, &desc->flags) || + !pinctrl_gpio_can_use_line(chip->base + info->line_offset)) + info->flags |= GPIOLINE_FLAG_KERNEL; + if (test_bit(FLAG_IS_OUT, &desc->flags)) + info->flags |= GPIOLINE_FLAG_IS_OUT; + if (test_bit(FLAG_ACTIVE_LOW, &desc->flags)) + info->flags |= GPIOLINE_FLAG_ACTIVE_LOW; + if (test_bit(FLAG_OPEN_DRAIN, &desc->flags)) + info->flags |= (GPIOLINE_FLAG_OPEN_DRAIN | + GPIOLINE_FLAG_IS_OUT); + if (test_bit(FLAG_OPEN_SOURCE, &desc->flags)) + info->flags |= (GPIOLINE_FLAG_OPEN_SOURCE | + GPIOLINE_FLAG_IS_OUT); + if (test_bit(FLAG_BIAS_DISABLE, &desc->flags)) + info->flags |= GPIOLINE_FLAG_BIAS_DISABLE; + if (test_bit(FLAG_PULL_DOWN, &desc->flags)) + info->flags |= GPIOLINE_FLAG_BIAS_PULL_DOWN; + if (test_bit(FLAG_PULL_UP, &desc->flags)) + info->flags |= GPIOLINE_FLAG_BIAS_PULL_UP; + + spin_unlock_irqrestore(&gpio_lock, flags); +} + /* * gpio_ioctl() - ioctl handler for the GPIO chardev */ @@ -1187,49 +1241,7 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) if (IS_ERR(desc)) return PTR_ERR(desc); - if (desc->name) { - strncpy(lineinfo.name, desc->name, - sizeof(lineinfo.name)); - lineinfo.name[sizeof(lineinfo.name)-1] = '\0'; - } else { - lineinfo.name[0] = '\0'; - } - if (desc->label) { - strncpy(lineinfo.consumer, desc->label, - sizeof(lineinfo.consumer)); - lineinfo.consumer[sizeof(lineinfo.consumer)-1] = '\0'; - } else { - lineinfo.consumer[0] = '\0'; - } - - /* - * Userspace only need to know that the kernel is using - * this GPIO so it can't use it. - */ - lineinfo.flags = 0; - if (test_bit(FLAG_REQUESTED, &desc->flags) || - test_bit(FLAG_IS_HOGGED, &desc->flags) || - test_bit(FLAG_USED_AS_IRQ, &desc->flags) || - test_bit(FLAG_EXPORT, &desc->flags) || - test_bit(FLAG_SYSFS, &desc->flags) || - !pinctrl_gpio_can_use_line(chip->base + lineinfo.line_offset)) - lineinfo.flags |= GPIOLINE_FLAG_KERNEL; - if (test_bit(FLAG_IS_OUT, &desc->flags)) - lineinfo.flags |= GPIOLINE_FLAG_IS_OUT; - if (test_bit(FLAG_ACTIVE_LOW, &desc->flags)) - lineinfo.flags |= GPIOLINE_FLAG_ACTIVE_LOW; - if (test_bit(FLAG_OPEN_DRAIN, &desc->flags)) - lineinfo.flags |= (GPIOLINE_FLAG_OPEN_DRAIN | - GPIOLINE_FLAG_IS_OUT); - if (test_bit(FLAG_OPEN_SOURCE, &desc->flags)) - lineinfo.flags |= (GPIOLINE_FLAG_OPEN_SOURCE | - GPIOLINE_FLAG_IS_OUT); - if (test_bit(FLAG_BIAS_DISABLE, &desc->flags)) - lineinfo.flags |= GPIOLINE_FLAG_BIAS_DISABLE; - if (test_bit(FLAG_PULL_DOWN, &desc->flags)) - lineinfo.flags |= GPIOLINE_FLAG_BIAS_PULL_DOWN; - if (test_bit(FLAG_PULL_UP, &desc->flags)) - lineinfo.flags |= GPIOLINE_FLAG_BIAS_PULL_UP; + gpio_desc_to_lineinfo(desc, &lineinfo); if (copy_to_user(ip, &lineinfo, sizeof(lineinfo))) return -EFAULT; From 51c1064e82e77b39a49889287ca50709303e2f26 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Fri, 22 Nov 2019 15:19:21 +0100 Subject: [PATCH 09/10] gpiolib: add new ioctl() for monitoring changes in line info Currently there is no way for user-space to be informed about changes in status of GPIO lines e.g. when someone else requests the line or its config changes. We can only periodically re-read the line-info. This is fine for simple one-off user-space tools, but any daemon that provides a centralized access to GPIO chips would benefit hugely from an event driven line info synchronization. This patch adds a new ioctl() that allows user-space processes to reuse the file descriptor associated with the character device for watching any changes in line properties. Every such event contains the updated line information. Currently the events are generated on three types of status changes: when a line is requested, when it's released and when its config is changed. The first two are self-explanatory. For the third one: this will only happen when another user-space process calls the new SET_CONFIG ioctl() as any changes that can happen from within the kernel (i.e. set_transitory() or set_debounce()) are of no interest to user-space. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij --- drivers/gpio/gpiolib.c | 188 ++++++++++++++++++++++++++++++++++++-- drivers/gpio/gpiolib.h | 1 + include/uapi/linux/gpio.h | 30 ++++++ 3 files changed, 210 insertions(+), 9 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 443321f9cf63..f73077f26eff 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -546,6 +546,9 @@ static long linehandle_set_config(struct linehandle_state *lh, if (ret) return ret; } + + atomic_notifier_call_chain(&desc->gdev->notifier, + GPIOLINE_CHANGED_CONFIG, desc); } return 0; } @@ -1201,14 +1204,25 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc, spin_unlock_irqrestore(&gpio_lock, flags); } +struct gpio_chardev_data { + struct gpio_device *gdev; + wait_queue_head_t wait; + DECLARE_KFIFO(events, struct gpioline_info_changed, 32); + struct notifier_block lineinfo_changed_nb; + unsigned long *watched_lines; +}; + /* * gpio_ioctl() - ioctl handler for the GPIO chardev */ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { - struct gpio_device *gdev = filp->private_data; + struct gpio_chardev_data *priv = filp->private_data; + struct gpio_device *gdev = priv->gdev; struct gpio_chip *chip = gdev->chip; void __user *ip = (void __user *)arg; + struct gpio_desc *desc; + __u32 offset; /* We fail any subsequent ioctl():s when the chip is gone */ if (!chip) @@ -1230,9 +1244,9 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) if (copy_to_user(ip, &chipinfo, sizeof(chipinfo))) return -EFAULT; return 0; - } else if (cmd == GPIO_GET_LINEINFO_IOCTL) { + } else if (cmd == GPIO_GET_LINEINFO_IOCTL || + cmd == GPIO_GET_LINEINFO_WATCH_IOCTL) { struct gpioline_info lineinfo; - struct gpio_desc *desc; if (copy_from_user(&lineinfo, ip, sizeof(lineinfo))) return -EFAULT; @@ -1245,11 +1259,25 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) if (copy_to_user(ip, &lineinfo, sizeof(lineinfo))) return -EFAULT; + + if (cmd == GPIO_GET_LINEINFO_WATCH_IOCTL) + set_bit(desc_to_gpio(desc), priv->watched_lines); + 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); + } else if (cmd == GPIO_GET_LINEINFO_UNWATCH_IOCTL) { + if (copy_from_user(&offset, ip, sizeof(offset))) + return -EFAULT; + + desc = gpiochip_get_desc(chip, offset); + if (IS_ERR(desc)) + return PTR_ERR(desc); + + clear_bit(desc_to_gpio(desc), &desc->flags); + return 0; } return -EINVAL; } @@ -1262,6 +1290,101 @@ static long gpio_ioctl_compat(struct file *filp, unsigned int cmd, } #endif +static struct gpio_chardev_data * +to_gpio_chardev_data(struct notifier_block *nb) +{ + return container_of(nb, struct gpio_chardev_data, lineinfo_changed_nb); +} + +static int lineinfo_changed_notify(struct notifier_block *nb, + unsigned long action, void *data) +{ + struct gpio_chardev_data *priv = to_gpio_chardev_data(nb); + struct gpioline_info_changed chg; + struct gpio_desc *desc = data; + int ret; + + if (!test_bit(desc_to_gpio(desc), priv->watched_lines)) + return NOTIFY_DONE; + + memset(&chg, 0, sizeof(chg)); + chg.info.line_offset = gpio_chip_hwgpio(desc); + chg.event_type = action; + chg.timestamp = ktime_get_ns(); + gpio_desc_to_lineinfo(desc, &chg.info); + + ret = kfifo_in_spinlocked(&priv->events, &chg, 1, &priv->wait.lock); + if (ret) + wake_up_poll(&priv->wait, EPOLLIN); + else + pr_debug_ratelimited("lineinfo event FIFO is full - event dropped\n"); + + return NOTIFY_OK; +} + +static __poll_t lineinfo_watch_poll(struct file *filep, + struct poll_table_struct *pollt) +{ + struct gpio_chardev_data *priv = filep->private_data; + __poll_t events = 0; + + poll_wait(filep, &priv->wait, pollt); + + if (!kfifo_is_empty_spinlocked_noirqsave(&priv->events, + &priv->wait.lock)) + events = EPOLLIN | EPOLLRDNORM; + + return events; +} + +static ssize_t lineinfo_watch_read(struct file *filep, char __user *buf, + size_t count, loff_t *off) +{ + struct gpio_chardev_data *priv = filep->private_data; + struct gpioline_info_changed event; + ssize_t bytes_read = 0; + int ret; + + if (count < sizeof(event)) + return -EINVAL; + + do { + spin_lock(&priv->wait.lock); + if (kfifo_is_empty(&priv->events)) { + if (bytes_read) { + spin_unlock(&priv->wait.lock); + return bytes_read; + } + + if (filep->f_flags & O_NONBLOCK) { + spin_unlock(&priv->wait.lock); + return -EAGAIN; + } + + ret = wait_event_interruptible_locked(priv->wait, + !kfifo_is_empty(&priv->events)); + if (ret) { + spin_unlock(&priv->wait.lock); + return ret; + } + } + + ret = kfifo_out(&priv->events, &event, 1); + spin_unlock(&priv->wait.lock); + if (ret != 1) { + ret = -EIO; + break; + /* We should never get here. See lineevent_read(). */ + } + + if (copy_to_user(buf + bytes_read, &event, sizeof(event))) + return -EFAULT; + bytes_read += sizeof(event); + } while (count >= bytes_read + sizeof(event)); + + return bytes_read; +} + /** * gpio_chrdev_open() - open the chardev for ioctl operations * @inode: inode for this chardev @@ -1272,14 +1395,48 @@ static int gpio_chrdev_open(struct inode *inode, struct file *filp) { struct gpio_device *gdev = container_of(inode->i_cdev, struct gpio_device, chrdev); + struct gpio_chardev_data *priv; + int ret = -ENOMEM; /* Fail on open if the backing gpiochip is gone */ if (!gdev->chip) return -ENODEV; - get_device(&gdev->dev); - filp->private_data = gdev; - return nonseekable_open(inode, filp); + priv = kzalloc(sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->watched_lines = bitmap_zalloc(gdev->chip->ngpio, GFP_KERNEL); + if (!priv->watched_lines) + goto out_free_priv; + + init_waitqueue_head(&priv->wait); + INIT_KFIFO(priv->events); + priv->gdev = gdev; + + priv->lineinfo_changed_nb.notifier_call = lineinfo_changed_notify; + ret = atomic_notifier_chain_register(&gdev->notifier, + &priv->lineinfo_changed_nb); + if (ret) + goto out_free_bitmap; + + get_device(&gdev->dev); + filp->private_data = priv; + + ret = nonseekable_open(inode, filp); + if (ret) + goto out_unregister_notifier; + + return ret; + +out_unregister_notifier: + atomic_notifier_chain_unregister(&gdev->notifier, + &priv->lineinfo_changed_nb); +out_free_bitmap: + bitmap_free(priv->watched_lines); +out_free_priv: + kfree(priv); + return ret; } /** @@ -1290,17 +1447,23 @@ static int gpio_chrdev_open(struct inode *inode, struct file *filp) */ static int gpio_chrdev_release(struct inode *inode, struct file *filp) { - struct gpio_device *gdev = container_of(inode->i_cdev, - struct gpio_device, chrdev); + struct gpio_chardev_data *priv = filp->private_data; + struct gpio_device *gdev = priv->gdev; + bitmap_free(priv->watched_lines); + atomic_notifier_chain_unregister(&gdev->notifier, + &priv->lineinfo_changed_nb); put_device(&gdev->dev); + kfree(priv); + return 0; } - static const struct file_operations gpio_fileops = { .release = gpio_chrdev_release, .open = gpio_chrdev_open, + .poll = lineinfo_watch_poll, + .read = lineinfo_watch_read, .owner = THIS_MODULE, .llseek = no_llseek, .unlocked_ioctl = gpio_ioctl, @@ -1511,6 +1674,8 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data, spin_unlock_irqrestore(&gpio_lock, flags); + ATOMIC_INIT_NOTIFIER_HEAD(&gdev->notifier); + #ifdef CONFIG_PINCTRL INIT_LIST_HEAD(&gdev->pin_ranges); #endif @@ -2843,6 +3008,8 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label) } done: spin_unlock_irqrestore(&gpio_lock, flags); + atomic_notifier_call_chain(&desc->gdev->notifier, + GPIOLINE_CHANGED_REQUESTED, desc); return ret; } @@ -2940,6 +3107,9 @@ static bool gpiod_free_commit(struct gpio_desc *desc) } spin_unlock_irqrestore(&gpio_lock, flags); + atomic_notifier_call_chain(&desc->gdev->notifier, + GPIOLINE_CHANGED_RELEASED, desc); + return ret; } diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h index 3e0aab2945d8..5ab90746b519 100644 --- a/drivers/gpio/gpiolib.h +++ b/drivers/gpio/gpiolib.h @@ -56,6 +56,7 @@ struct gpio_device { const char *label; void *data; struct list_head list; + struct atomic_notifier_head notifier; #ifdef CONFIG_PINCTRL /* diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h index 799cf823d493..dca320764e4d 100644 --- a/include/uapi/linux/gpio.h +++ b/include/uapi/linux/gpio.h @@ -59,6 +59,34 @@ struct gpioline_info { /* Maximum number of requested handles */ #define GPIOHANDLES_MAX 64 +/* Possible line status change events */ +enum { + GPIOLINE_CHANGED_REQUESTED = 1, + GPIOLINE_CHANGED_RELEASED, + GPIOLINE_CHANGED_CONFIG, +}; + +/** + * struct gpioline_info_changed - Information about a change in status + * of a GPIO line + * @info: updated line information + * @timestamp: estimate of time of status change occurrence, in nanoseconds + * and GPIOLINE_CHANGED_CONFIG + * @event_type: one of GPIOLINE_CHANGED_REQUESTED, GPIOLINE_CHANGED_RELEASED + * + * Note: struct gpioline_info embedded here has 32-bit alignment on its own, + * but it works fine with 64-bit alignment too. With its 72 byte size, we can + * guarantee there are no implicit holes between it and subsequent members. + * The 20-byte padding at the end makes sure we don't add any implicit padding + * at the end of the structure on 64-bit architectures. + */ +struct gpioline_info_changed { + struct gpioline_info info; + __u64 timestamp; + __u32 event_type; + __u32 padding[5]; /* for future use */ +}; + /* Linerequest flags */ #define GPIOHANDLE_REQUEST_INPUT (1UL << 0) #define GPIOHANDLE_REQUEST_OUTPUT (1UL << 1) @@ -176,6 +204,8 @@ struct gpioevent_data { #define GPIO_GET_CHIPINFO_IOCTL _IOR(0xB4, 0x01, struct gpiochip_info) #define GPIO_GET_LINEINFO_IOCTL _IOWR(0xB4, 0x02, struct gpioline_info) +#define GPIO_GET_LINEINFO_WATCH_IOCTL _IOWR(0xB4, 0x0b, struct gpioline_info) +#define GPIO_GET_LINEINFO_UNWATCH_IOCTL _IOWR(0xB4, 0x0c, __u32) #define GPIO_GET_LINEHANDLE_IOCTL _IOWR(0xB4, 0x03, struct gpiohandle_request) #define GPIO_GET_LINEEVENT_IOCTL _IOWR(0xB4, 0x04, struct gpioevent_request) From 33f0c47b8fb4724792b16351a32b24902a5d3b07 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 27 Nov 2019 10:17:54 +0100 Subject: [PATCH 10/10] tools: gpio: implement gpio-watch Add a simple program that allows to test the new LINECHANGED_FD ioctl(). Signed-off-by: Bartosz Golaszewski Reviewed-by: Andy Shevchenko --- tools/gpio/.gitignore | 1 + tools/gpio/Build | 1 + tools/gpio/Makefile | 11 ++++- tools/gpio/gpio-watch.c | 99 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 tools/gpio/gpio-watch.c diff --git a/tools/gpio/.gitignore b/tools/gpio/.gitignore index a94c0e83b209..eab36c6d7751 100644 --- a/tools/gpio/.gitignore +++ b/tools/gpio/.gitignore @@ -1,4 +1,5 @@ gpio-event-mon gpio-hammer +gpio-watch lsgpio include/linux/gpio.h diff --git a/tools/gpio/Build b/tools/gpio/Build index 4141f35837db..67c7b7f6a717 100644 --- a/tools/gpio/Build +++ b/tools/gpio/Build @@ -2,3 +2,4 @@ gpio-utils-y += gpio-utils.o lsgpio-y += lsgpio.o gpio-utils.o gpio-hammer-y += gpio-hammer.o gpio-utils.o gpio-event-mon-y += gpio-event-mon.o gpio-utils.o +gpio-watch-y += gpio-watch.o diff --git a/tools/gpio/Makefile b/tools/gpio/Makefile index 6080de58861f..842287e42c83 100644 --- a/tools/gpio/Makefile +++ b/tools/gpio/Makefile @@ -18,7 +18,7 @@ MAKEFLAGS += -r override CFLAGS += -O2 -Wall -g -D_GNU_SOURCE -I$(OUTPUT)include -ALL_TARGETS := lsgpio gpio-hammer gpio-event-mon +ALL_TARGETS := lsgpio gpio-hammer gpio-event-mon gpio-watch ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS)) all: $(ALL_PROGRAMS) @@ -66,6 +66,15 @@ $(GPIO_EVENT_MON_IN): prepare FORCE $(OUTPUT)gpio-utils-in.o $(OUTPUT)gpio-event-mon: $(GPIO_EVENT_MON_IN) $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $< -o $@ +# +# gpio-watch +# +GPIO_WATCH_IN := $(OUTPUT)gpio-watch-in.o +$(GPIO_WATCH_IN): prepare FORCE + $(Q)$(MAKE) $(build)=gpio-watch +$(OUTPUT)gpio-watch: $(GPIO_WATCH_IN) + $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $< -o $@ + clean: rm -f $(ALL_PROGRAMS) rm -f $(OUTPUT)include/linux/gpio.h diff --git a/tools/gpio/gpio-watch.c b/tools/gpio/gpio-watch.c new file mode 100644 index 000000000000..5cea24fddfa7 --- /dev/null +++ b/tools/gpio/gpio-watch.c @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * gpio-watch - monitor unrequested lines for property changes using the + * character device + * + * Copyright (C) 2019 BayLibre SAS + * Author: Bartosz Golaszewski + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +int main(int argc, char **argv) +{ + struct gpioline_info_changed chg; + struct gpioline_info req; + struct pollfd pfd; + int fd, i, j, ret; + char *event, *end; + ssize_t rd; + + if (argc < 3) + goto err_usage; + + fd = open(argv[1], O_RDWR | O_CLOEXEC); + if (fd < 0) { + perror("unable to open gpiochip"); + return EXIT_FAILURE; + } + + for (i = 0, j = 2; i < argc - 2; i++, j++) { + memset(&req, 0, sizeof(req)); + + req.line_offset = strtoul(argv[j], &end, 0); + if (*end != '\0') + goto err_usage; + + ret = ioctl(fd, GPIO_GET_LINEINFO_WATCH_IOCTL, &req); + if (ret) { + perror("unable to set up line watch"); + return EXIT_FAILURE; + } + } + + pfd.fd = fd; + pfd.events = POLLIN | POLLPRI; + + for (;;) { + ret = poll(&pfd, 1, 5000); + if (ret < 0) { + perror("error polling the linechanged fd"); + return EXIT_FAILURE; + } else if (ret > 0) { + memset(&chg, 0, sizeof(chg)); + rd = read(pfd.fd, &chg, sizeof(chg)); + if (rd < 0 || rd != sizeof(chg)) { + if (rd != sizeof(chg)) + errno = EIO; + + perror("error reading line change event"); + return EXIT_FAILURE; + } + + switch (chg.event_type) { + case GPIOLINE_CHANGED_REQUESTED: + event = "requested"; + break; + case GPIOLINE_CHANGED_RELEASED: + event = "released"; + break; + case GPIOLINE_CHANGED_CONFIG: + event = "config changed"; + break; + default: + fprintf(stderr, + "invalid event type received from the kernel\n"); + return EXIT_FAILURE; + } + + printf("line %u: %s at %llu\n", + chg.info.line_offset, event, chg.timestamp); + } + } + + return 0; + +err_usage: + printf("%s: ...\n", argv[0]); + return EXIT_FAILURE; +}