[PATCH 00/17] gpiolib: cdev: pre-uAPI v2 cleanups

[PATCH 00/17] gpiolib: cdev: pre-uAPI v2 cleanups

[Kent Gibson]

This collection of patches provides improvements to or
address minor problems in gpiolib-cdev.

The majority of the patches (1-7, 9-11) have been pulled directly from
my "gpio: cdev: add uAPI V2" patch set, as they are not related to any
uAPI changes.
The remaining patches were either split out of the remaining patches
from that set, as they are not directly part of the uAPI changes, or
were inspired by fixes for issues in that set, or were only noticed
subsequent to that set.

Changes since "gpio: cdev: add uAPI V2":
 - rebase onto latest gpio/devel
 - fix typo in patch 1 commit description
 - replace patch 8 with the blocking notifier call chain patch
 - rename priv to cdev instead of gcdev in patch 9
 - fix error handling in patch 10
 - add patches 12 to 17

Kent Gibson (17):
  gpiolib: move gpiolib-sysfs function declarations into their own
    header
  gpiolib: cdev: sort includes
  gpiolib: cdev: minor indentation fixes
  gpiolib: cdev: refactor gpiohandle_flags_to_desc_flags
  gpiolib: cdev: rename 'filep' and 'filp' to 'file' to be consistent
    with other use
  gpiolib: cdev: rename numdescs to num_descs
  gpiolib: cdev: remove pointless decrement of i
  gpiolib: cdev: use blocking notifier call chain instead of atomic
  gpiolib: cdev: rename priv to cdev
  gpiolib: cdev: fix minor race in GET_LINEINFO_WATCH
  gpiolib: cdev: remove recalculation of offset
  gpiolib: cdev: refactor linehandle cleanup into linehandle_free
  gpiolib: cdev: refactor lineevent cleanup into lineevent_free
  gpio: uapi: fix misplaced comment line
  tools: gpio: fix spurious close warning in lsgpio
  tools: gpio: fix spurious close warning in gpio-utils
  tools: gpio: fix spurious close warning in gpio-event-mon

 drivers/gpio/gpiolib-cdev.c  | 385 ++++++++++++++++-------------------
 drivers/gpio/gpiolib-sysfs.c |   1 +
 drivers/gpio/gpiolib-sysfs.h |  24 +++
 drivers/gpio/gpiolib.c       |  15 +-
 drivers/gpio/gpiolib.h       |  20 +-
 include/uapi/linux/gpio.h    |   2 +-
 tools/gpio/gpio-event-mon.c  |   3 +-
 tools/gpio/gpio-utils.c      |   4 +-
 tools/gpio/lsgpio.c          |   3 +-
 9 files changed, 217 insertions(+), 240 deletions(-)
 create mode 100644 drivers/gpio/gpiolib-sysfs.h


base-commit: b239e4454e59bc85d466eb5630da46f6a876df77
-- 
2.27.0

[PATCH 01/17] gpiolib: move gpiolib-sysfs function declarations into their own header

[Kent Gibson]

Move gpiolib-sysfs function declarations into their own header.

These functions are in gpiolib-sysfs.c, and are only required by gpiolib.c,
and so should be in a module header, not gpiolib.h.

This brings gpiolib-sysfs into line with gpiolib-cdev, and is another step
towards removing the sysfs inferface.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-sysfs.c |  1 +
 drivers/gpio/gpiolib-sysfs.h | 24 ++++++++++++++++++++++++
 drivers/gpio/gpiolib.c       |  1 +
 drivers/gpio/gpiolib.h       | 18 ------------------
 4 files changed, 26 insertions(+), 18 deletions(-)
 create mode 100644 drivers/gpio/gpiolib-sysfs.h

diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 82371fe2ccc6..728f6c687182 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -11,6 +11,7 @@
 #include <linux/ctype.h>
 
 #include "gpiolib.h"
+#include "gpiolib-sysfs.h"
 
 #define GPIO_IRQF_TRIGGER_FALLING	BIT(0)
 #define GPIO_IRQF_TRIGGER_RISING	BIT(1)
diff --git a/drivers/gpio/gpiolib-sysfs.h b/drivers/gpio/gpiolib-sysfs.h
new file mode 100644
index 000000000000..ddd0e503f8eb
--- /dev/null
+++ b/drivers/gpio/gpiolib-sysfs.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef GPIOLIB_SYSFS_H
+#define GPIOLIB_SYSFS_H
+
+#ifdef CONFIG_GPIO_SYSFS
+
+int gpiochip_sysfs_register(struct gpio_device *gdev);
+void gpiochip_sysfs_unregister(struct gpio_device *gdev);
+
+#else
+
+static inline int gpiochip_sysfs_register(struct gpio_device *gdev)
+{
+	return 0;
+}
+
+static inline void gpiochip_sysfs_unregister(struct gpio_device *gdev)
+{
+}
+
+#endif /* CONFIG_GPIO_SYSFS */
+
+#endif /* GPIOLIB_SYSFS_H */
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 291c088a5964..4d267c69482c 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -26,6 +26,7 @@
 #include "gpiolib-of.h"
 #include "gpiolib-acpi.h"
 #include "gpiolib-cdev.h"
+#include "gpiolib-sysfs.h"
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/gpio.h>
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 9ed242316414..2dee4e1e12dc 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -175,22 +175,4 @@ static inline int gpio_chip_hwgpio(const struct gpio_desc *desc)
 #define chip_dbg(gc, fmt, ...)					\
 	dev_dbg(&gc->gpiodev->dev, "(%s): " fmt, gc->label, ##__VA_ARGS__)
 
-#ifdef CONFIG_GPIO_SYSFS
-
-int gpiochip_sysfs_register(struct gpio_device *gdev);
-void gpiochip_sysfs_unregister(struct gpio_device *gdev);
-
-#else
-
-static inline int gpiochip_sysfs_register(struct gpio_device *gdev)
-{
-	return 0;
-}
-
-static inline void gpiochip_sysfs_unregister(struct gpio_device *gdev)
-{
-}
-
-#endif /* CONFIG_GPIO_SYSFS */
-
 #endif /* GPIOLIB_H */
-- 
2.27.0

[PATCH 02/17] gpiolib: cdev: sort includes

[Kent Gibson]

Sort the includes of gpiolib-cdev.c to make it easier to identify if a
module is included and to avoid duplication.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-cdev.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index b8b872724628..55a9b7b44304 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -1,24 +1,24 @@
 // SPDX-License-Identifier: GPL-2.0
 
+#include <linux/anon_inodes.h>
 #include <linux/bitmap.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/interrupt.h>
-#include <linux/irqreturn.h>
-#include <linux/spinlock.h>
+#include <linux/cdev.h>
+#include <linux/compat.h>
 #include <linux/device.h>
 #include <linux/err.h>
+#include <linux/file.h>
 #include <linux/gpio.h>
 #include <linux/gpio/driver.h>
-#include <linux/pinctrl/consumer.h>
-#include <linux/cdev.h>
-#include <linux/uaccess.h>
-#include <linux/compat.h>
-#include <linux/anon_inodes.h>
-#include <linux/file.h>
+#include <linux/interrupt.h>
+#include <linux/irqreturn.h>
+#include <linux/kernel.h>
 #include <linux/kfifo.h>
+#include <linux/module.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/poll.h>
+#include <linux/spinlock.h>
 #include <linux/timekeeping.h>
+#include <linux/uaccess.h>
 #include <uapi/linux/gpio.h>
 
 #include "gpiolib.h"
-- 
2.27.0

[PATCH 03/17] gpiolib: cdev: minor indentation fixes

[Kent Gibson]

Make indentation consistent with other use to improve readability.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-cdev.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 55a9b7b44304..889ed2dc9e58 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -98,7 +98,7 @@ static int linehandle_validate_flags(u32 flags)
 	/* Only one bias flag can be set. */
 	if (((flags & GPIOHANDLE_REQUEST_BIAS_DISABLE) &&
 	     (flags & (GPIOHANDLE_REQUEST_BIAS_PULL_DOWN |
-			GPIOHANDLE_REQUEST_BIAS_PULL_UP))) ||
+		       GPIOHANDLE_REQUEST_BIAS_PULL_UP))) ||
 	    ((flags & GPIOHANDLE_REQUEST_BIAS_PULL_DOWN) &&
 	     (flags & GPIOHANDLE_REQUEST_BIAS_PULL_UP)))
 		return -EINVAL;
@@ -212,11 +212,11 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
 
 		/* Reuse the array setting function */
 		return gpiod_set_array_value_complex(false,
-					      true,
-					      lh->numdescs,
-					      lh->descs,
-					      NULL,
-					      vals);
+						     true,
+						     lh->numdescs,
+						     lh->descs,
+						     NULL,
+						     vals);
 	} else if (cmd == GPIOHANDLE_SET_CONFIG_IOCTL) {
 		return linehandle_set_config(lh, ip);
 	}
@@ -225,7 +225,7 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
 
 #ifdef CONFIG_COMPAT
 static long linehandle_ioctl_compat(struct file *filep, unsigned int cmd,
-			     unsigned long arg)
+				    unsigned long arg)
 {
 	return linehandle_ioctl(filep, cmd, (unsigned long)compat_ptr(arg));
 }
@@ -428,7 +428,7 @@ struct lineevent_state {
 	GPIOEVENT_REQUEST_FALLING_EDGE)
 
 static __poll_t lineevent_poll(struct file *filep,
-				   struct poll_table_struct *wait)
+			       struct poll_table_struct *wait)
 {
 	struct lineevent_state *le = filep->private_data;
 	__poll_t events = 0;
@@ -720,11 +720,11 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 
 	/* Request a thread to read the events */
 	ret = request_threaded_irq(le->irq,
-			lineevent_irq_handler,
-			lineevent_irq_thread,
-			irqflags,
-			le->label,
-			le);
+				   lineevent_irq_handler,
+				   lineevent_irq_thread,
+				   irqflags,
+				   le->label,
+				   le);
 	if (ret)
 		goto out_free_desc;
 
@@ -1052,7 +1052,7 @@ static ssize_t lineinfo_watch_read(struct file *filep, char __user *buf,
 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_device, chrdev);
 	struct gpio_chardev_data *priv;
 	int ret = -ENOMEM;
 
-- 
2.27.0

[PATCH 04/17] gpiolib: cdev: refactor gpiohandle_flags_to_desc_flags

[Kent Gibson]

Refactor the mapping from handle flags to desc flags into a helper
function.

The assign_bit is overkill where it is replacing the set_bit cases, as is
rechecking bits known to be clear in some circumstances, but the DRY
simplification more than makes up for any performance degradation,
especially as this is not a hot path.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-cdev.c | 60 ++++++++++++-------------------------
 1 file changed, 19 insertions(+), 41 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 889ed2dc9e58..e64613b8d0ba 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -106,6 +106,22 @@ static int linehandle_validate_flags(u32 flags)
 	return 0;
 }
 
+static void linehandle_flags_to_desc_flags(u32 lflags, unsigned long *flagsp)
+{
+	assign_bit(FLAG_ACTIVE_LOW, flagsp,
+		   lflags & GPIOHANDLE_REQUEST_ACTIVE_LOW);
+	assign_bit(FLAG_OPEN_DRAIN, flagsp,
+		   lflags & GPIOHANDLE_REQUEST_OPEN_DRAIN);
+	assign_bit(FLAG_OPEN_SOURCE, flagsp,
+		   lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE);
+	assign_bit(FLAG_PULL_UP, flagsp,
+		   lflags & GPIOHANDLE_REQUEST_BIAS_PULL_UP);
+	assign_bit(FLAG_PULL_DOWN, flagsp,
+		   lflags & GPIOHANDLE_REQUEST_BIAS_PULL_DOWN);
+	assign_bit(FLAG_BIAS_DISABLE, flagsp,
+		   lflags & GPIOHANDLE_REQUEST_BIAS_DISABLE);
+}
+
 static long linehandle_set_config(struct linehandle_state *lh,
 				  void __user *ip)
 {
@@ -113,7 +129,6 @@ static long linehandle_set_config(struct linehandle_state *lh,
 	struct gpio_desc *desc;
 	int i, ret;
 	u32 lflags;
-	unsigned long *flagsp;
 
 	if (copy_from_user(&gcnf, ip, sizeof(gcnf)))
 		return -EFAULT;
@@ -125,25 +140,7 @@ static long linehandle_set_config(struct linehandle_state *lh,
 
 	for (i = 0; i < lh->numdescs; i++) {
 		desc = lh->descs[i];
-		flagsp = &desc->flags;
-
-		assign_bit(FLAG_ACTIVE_LOW, flagsp,
-			lflags & GPIOHANDLE_REQUEST_ACTIVE_LOW);
-
-		assign_bit(FLAG_OPEN_DRAIN, flagsp,
-			lflags & GPIOHANDLE_REQUEST_OPEN_DRAIN);
-
-		assign_bit(FLAG_OPEN_SOURCE, flagsp,
-			lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE);
-
-		assign_bit(FLAG_PULL_UP, flagsp,
-			lflags & GPIOHANDLE_REQUEST_BIAS_PULL_UP);
-
-		assign_bit(FLAG_PULL_DOWN, flagsp,
-			lflags & GPIOHANDLE_REQUEST_BIAS_PULL_DOWN);
-
-		assign_bit(FLAG_BIAS_DISABLE, flagsp,
-			lflags & GPIOHANDLE_REQUEST_BIAS_DISABLE);
+		linehandle_flags_to_desc_flags(gcnf.flags, &desc->flags);
 
 		/*
 		 * Lines have to be requested explicitly for input
@@ -306,19 +303,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 			goto out_free_descs;
 		lh->descs[i] = desc;
 		count = i + 1;
-
-		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);
-		if (lflags & GPIOHANDLE_REQUEST_BIAS_DISABLE)
-			set_bit(FLAG_BIAS_DISABLE, &desc->flags);
-		if (lflags & GPIOHANDLE_REQUEST_BIAS_PULL_DOWN)
-			set_bit(FLAG_PULL_DOWN, &desc->flags);
-		if (lflags & GPIOHANDLE_REQUEST_BIAS_PULL_UP)
-			set_bit(FLAG_PULL_UP, &desc->flags);
+		linehandle_flags_to_desc_flags(handlereq.flags, &desc->flags);
 
 		ret = gpiod_set_transitory(desc, false);
 		if (ret < 0)
@@ -685,14 +670,7 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 	le->desc = desc;
 	le->eflags = eflags;
 
-	if (lflags & GPIOHANDLE_REQUEST_ACTIVE_LOW)
-		set_bit(FLAG_ACTIVE_LOW, &desc->flags);
-	if (lflags & GPIOHANDLE_REQUEST_BIAS_DISABLE)
-		set_bit(FLAG_BIAS_DISABLE, &desc->flags);
-	if (lflags & GPIOHANDLE_REQUEST_BIAS_PULL_DOWN)
-		set_bit(FLAG_PULL_DOWN, &desc->flags);
-	if (lflags & GPIOHANDLE_REQUEST_BIAS_PULL_UP)
-		set_bit(FLAG_PULL_UP, &desc->flags);
+	linehandle_flags_to_desc_flags(lflags, &desc->flags);
 
 	ret = gpiod_direction_input(desc);
 	if (ret)
-- 
2.27.0

[PATCH 05/17] gpiolib: cdev: rename 'filep' and 'filp' to 'file' to be consistent with other use

[Kent Gibson]

Rename 'filep' and 'filp' to 'file' to be consistent with other use
and improve readability.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-cdev.c | 70 ++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index e64613b8d0ba..0d3a799e09ae 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -164,10 +164,10 @@ static long linehandle_set_config(struct linehandle_state *lh,
 	return 0;
 }
 
-static long linehandle_ioctl(struct file *filep, unsigned int cmd,
+static long linehandle_ioctl(struct file *file, unsigned int cmd,
 			     unsigned long arg)
 {
-	struct linehandle_state *lh = filep->private_data;
+	struct linehandle_state *lh = file->private_data;
 	void __user *ip = (void __user *)arg;
 	struct gpiohandle_data ghd;
 	DECLARE_BITMAP(vals, GPIOHANDLES_MAX);
@@ -221,16 +221,16 @@ static long linehandle_ioctl(struct file *filep, unsigned int cmd,
 }
 
 #ifdef CONFIG_COMPAT
-static long linehandle_ioctl_compat(struct file *filep, unsigned int cmd,
+static long linehandle_ioctl_compat(struct file *file, unsigned int cmd,
 				    unsigned long arg)
 {
-	return linehandle_ioctl(filep, cmd, (unsigned long)compat_ptr(arg));
+	return linehandle_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
 }
 #endif
 
-static int linehandle_release(struct inode *inode, struct file *filep)
+static int linehandle_release(struct inode *inode, struct file *file)
 {
-	struct linehandle_state *lh = filep->private_data;
+	struct linehandle_state *lh = file->private_data;
 	struct gpio_device *gdev = lh->gdev;
 	int i;
 
@@ -412,13 +412,13 @@ struct lineevent_state {
 	(GPIOEVENT_REQUEST_RISING_EDGE | \
 	GPIOEVENT_REQUEST_FALLING_EDGE)
 
-static __poll_t lineevent_poll(struct file *filep,
+static __poll_t lineevent_poll(struct file *file,
 			       struct poll_table_struct *wait)
 {
-	struct lineevent_state *le = filep->private_data;
+	struct lineevent_state *le = file->private_data;
 	__poll_t events = 0;
 
-	poll_wait(filep, &le->wait, wait);
+	poll_wait(file, &le->wait, wait);
 
 	if (!kfifo_is_empty_spinlocked_noirqsave(&le->events, &le->wait.lock))
 		events = EPOLLIN | EPOLLRDNORM;
@@ -427,12 +427,12 @@ static __poll_t lineevent_poll(struct file *filep,
 }
 
 
-static ssize_t lineevent_read(struct file *filep,
+static ssize_t lineevent_read(struct file *file,
 			      char __user *buf,
 			      size_t count,
 			      loff_t *f_ps)
 {
-	struct lineevent_state *le = filep->private_data;
+	struct lineevent_state *le = file->private_data;
 	struct gpioevent_data ge;
 	ssize_t bytes_read = 0;
 	int ret;
@@ -448,7 +448,7 @@ static ssize_t lineevent_read(struct file *filep,
 				return bytes_read;
 			}
 
-			if (filep->f_flags & O_NONBLOCK) {
+			if (file->f_flags & O_NONBLOCK) {
 				spin_unlock(&le->wait.lock);
 				return -EAGAIN;
 			}
@@ -481,9 +481,9 @@ static ssize_t lineevent_read(struct file *filep,
 	return bytes_read;
 }
 
-static int lineevent_release(struct inode *inode, struct file *filep)
+static int lineevent_release(struct inode *inode, struct file *file)
 {
-	struct lineevent_state *le = filep->private_data;
+	struct lineevent_state *le = file->private_data;
 	struct gpio_device *gdev = le->gdev;
 
 	free_irq(le->irq, le);
@@ -494,10 +494,10 @@ static int lineevent_release(struct inode *inode, struct file *filep)
 	return 0;
 }
 
-static long lineevent_ioctl(struct file *filep, unsigned int cmd,
+static long lineevent_ioctl(struct file *file, unsigned int cmd,
 			    unsigned long arg)
 {
-	struct lineevent_state *le = filep->private_data;
+	struct lineevent_state *le = file->private_data;
 	void __user *ip = (void __user *)arg;
 	struct gpiohandle_data ghd;
 
@@ -524,10 +524,10 @@ static long lineevent_ioctl(struct file *filep, unsigned int cmd,
 }
 
 #ifdef CONFIG_COMPAT
-static long lineevent_ioctl_compat(struct file *filep, unsigned int cmd,
+static long lineevent_ioctl_compat(struct file *file, unsigned int cmd,
 				   unsigned long arg)
 {
-	return lineevent_ioctl(filep, cmd, (unsigned long)compat_ptr(arg));
+	return lineevent_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
 }
 #endif
 
@@ -826,9 +826,9 @@ struct gpio_chardev_data {
 /*
  * gpio_ioctl() - ioctl handler for the GPIO chardev
  */
-static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
-	struct gpio_chardev_data *priv = filp->private_data;
+	struct gpio_chardev_data *priv = file->private_data;
 	struct gpio_device *gdev = priv->gdev;
 	struct gpio_chip *gc = gdev->chip;
 	void __user *ip = (void __user *)arg;
@@ -919,10 +919,10 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 }
 
 #ifdef CONFIG_COMPAT
-static long gpio_ioctl_compat(struct file *filp, unsigned int cmd,
+static long gpio_ioctl_compat(struct file *file, unsigned int cmd,
 			      unsigned long arg)
 {
-	return gpio_ioctl(filp, cmd, (unsigned long)compat_ptr(arg));
+	return gpio_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
 }
 #endif
 
@@ -958,13 +958,13 @@ static int lineinfo_changed_notify(struct notifier_block *nb,
 	return NOTIFY_OK;
 }
 
-static __poll_t lineinfo_watch_poll(struct file *filep,
+static __poll_t lineinfo_watch_poll(struct file *file,
 				    struct poll_table_struct *pollt)
 {
-	struct gpio_chardev_data *priv = filep->private_data;
+	struct gpio_chardev_data *priv = file->private_data;
 	__poll_t events = 0;
 
-	poll_wait(filep, &priv->wait, pollt);
+	poll_wait(file, &priv->wait, pollt);
 
 	if (!kfifo_is_empty_spinlocked_noirqsave(&priv->events,
 						 &priv->wait.lock))
@@ -973,10 +973,10 @@ static __poll_t lineinfo_watch_poll(struct file *filep,
 	return events;
 }
 
-static ssize_t lineinfo_watch_read(struct file *filep, char __user *buf,
+static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
 				   size_t count, loff_t *off)
 {
-	struct gpio_chardev_data *priv = filep->private_data;
+	struct gpio_chardev_data *priv = file->private_data;
 	struct gpioline_info_changed event;
 	ssize_t bytes_read = 0;
 	int ret;
@@ -992,7 +992,7 @@ static ssize_t lineinfo_watch_read(struct file *filep, char __user *buf,
 				return bytes_read;
 			}
 
-			if (filep->f_flags & O_NONBLOCK) {
+			if (file->f_flags & O_NONBLOCK) {
 				spin_unlock(&priv->wait.lock);
 				return -EAGAIN;
 			}
@@ -1024,10 +1024,10 @@ static ssize_t lineinfo_watch_read(struct file *filep, char __user *buf,
 /**
  * gpio_chrdev_open() - open the chardev for ioctl operations
  * @inode: inode for this chardev
- * @filp: file struct for storing private data
+ * @file: file struct for storing private data
  * Returns 0 on success
  */
-static int gpio_chrdev_open(struct inode *inode, struct file *filp)
+static int gpio_chrdev_open(struct inode *inode, struct file *file)
 {
 	struct gpio_device *gdev = container_of(inode->i_cdev,
 						struct gpio_device, chrdev);
@@ -1057,9 +1057,9 @@ static int gpio_chrdev_open(struct inode *inode, struct file *filp)
 		goto out_free_bitmap;
 
 	get_device(&gdev->dev);
-	filp->private_data = priv;
+	file->private_data = priv;
 
-	ret = nonseekable_open(inode, filp);
+	ret = nonseekable_open(inode, file);
 	if (ret)
 		goto out_unregister_notifier;
 
@@ -1078,12 +1078,12 @@ static int gpio_chrdev_open(struct inode *inode, struct file *filp)
 /**
  * gpio_chrdev_release() - close chardev after ioctl operations
  * @inode: inode for this chardev
- * @filp: file struct for storing private data
+ * @file: file struct for storing private data
  * Returns 0 on success
  */
-static int gpio_chrdev_release(struct inode *inode, struct file *filp)
+static int gpio_chrdev_release(struct inode *inode, struct file *file)
 {
-	struct gpio_chardev_data *priv = filp->private_data;
+	struct gpio_chardev_data *priv = file->private_data;
 	struct gpio_device *gdev = priv->gdev;
 
 	bitmap_free(priv->watched_lines);
-- 
2.27.0

[PATCH 06/17] gpiolib: cdev: rename numdescs to num_descs

[Kent Gibson]

Rename numdescs to num_descs to be more consistent with the naming of
other counters and improve readability.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-cdev.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 0d3a799e09ae..b39e7ef8c0d4 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -39,13 +39,13 @@
  * @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
+ * @num_descs: 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;
+	u32 num_descs;
 };
 
 #define GPIOHANDLE_REQUEST_VALID_FLAGS \
@@ -138,7 +138,7 @@ static long linehandle_set_config(struct linehandle_state *lh,
 	if (ret)
 		return ret;
 
-	for (i = 0; i < lh->numdescs; i++) {
+	for (i = 0; i < lh->num_descs; i++) {
 		desc = lh->descs[i];
 		linehandle_flags_to_desc_flags(gcnf.flags, &desc->flags);
 
@@ -177,7 +177,7 @@ static long linehandle_ioctl(struct file *file, unsigned int cmd,
 		/* NOTE: It's ok to read values of output lines. */
 		int ret = gpiod_get_array_value_complex(false,
 							true,
-							lh->numdescs,
+							lh->num_descs,
 							lh->descs,
 							NULL,
 							vals);
@@ -185,7 +185,7 @@ static long linehandle_ioctl(struct file *file, unsigned int cmd,
 			return ret;
 
 		memset(&ghd, 0, sizeof(ghd));
-		for (i = 0; i < lh->numdescs; i++)
+		for (i = 0; i < lh->num_descs; i++)
 			ghd.values[i] = test_bit(i, vals);
 
 		if (copy_to_user(ip, &ghd, sizeof(ghd)))
@@ -204,13 +204,13 @@ static long linehandle_ioctl(struct file *file, unsigned int cmd,
 			return -EFAULT;
 
 		/* Clamp all values to [0,1] */
-		for (i = 0; i < lh->numdescs; i++)
+		for (i = 0; i < lh->num_descs; i++)
 			__assign_bit(i, vals, ghd.values[i]);
 
 		/* Reuse the array setting function */
 		return gpiod_set_array_value_complex(false,
 						     true,
-						     lh->numdescs,
+						     lh->num_descs,
 						     lh->descs,
 						     NULL,
 						     vals);
@@ -234,7 +234,7 @@ static int linehandle_release(struct inode *inode, struct file *file)
 	struct gpio_device *gdev = lh->gdev;
 	int i;
 
-	for (i = 0; i < lh->numdescs; i++)
+	for (i = 0; i < lh->num_descs; i++)
 		gpiod_free(lh->descs[i]);
 	kfree(lh->label);
 	kfree(lh);
@@ -333,7 +333,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 	}
 	/* Let i point at the last handle */
 	i--;
-	lh->numdescs = handlereq.lines;
+	lh->num_descs = handlereq.lines;
 
 	fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
 	if (fd < 0) {
@@ -364,7 +364,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 	fd_install(fd, file);
 
 	dev_dbg(&gdev->dev, "registered chardev handle for %d lines\n",
-		lh->numdescs);
+		lh->num_descs);
 
 	return 0;
 
-- 
2.27.0

[PATCH 07/17] gpiolib: cdev: remove pointless decrement of i

[Kent Gibson]

Remove pointless decrement of variable, and associated comment.

While i is used subsequently, it is re-initialized so this decrement
serves no purpose.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-cdev.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index b39e7ef8c0d4..d50339ef6f05 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -331,8 +331,6 @@ 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->num_descs = handlereq.lines;
 
 	fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
-- 
2.27.0

[PATCH 08/17] gpiolib: cdev: use blocking notifier call chain instead of atomic

[Kent Gibson]

Replace usage of atomic_notifier_call_chain with
blocking_notifier_call_chain as the notifier function,
lineinfo_changed_notify, calls gpio_desc_to_lineinfo,
which calls pinctrl_gpio_can_use_line, which can sleep.

The chain isn't being called from an atomic context so the
the blocking notifier is a suitable substitute.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-cdev.c | 24 ++++++++++++------------
 drivers/gpio/gpiolib.c      | 14 +++++++-------
 drivers/gpio/gpiolib.h      |  2 +-
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index d50339ef6f05..352d815bbd07 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -158,8 +158,8 @@ static long linehandle_set_config(struct linehandle_state *lh,
 				return ret;
 		}
 
-		atomic_notifier_call_chain(&desc->gdev->notifier,
-					   GPIOLINE_CHANGED_CONFIG, desc);
+		blocking_notifier_call_chain(&desc->gdev->notifier,
+					     GPIOLINE_CHANGED_CONFIG, desc);
 	}
 	return 0;
 }
@@ -325,8 +325,8 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 				goto out_free_descs;
 		}
 
-		atomic_notifier_call_chain(&desc->gdev->notifier,
-					   GPIOLINE_CHANGED_REQUESTED, desc);
+		blocking_notifier_call_chain(&desc->gdev->notifier,
+					     GPIOLINE_CHANGED_REQUESTED, desc);
 
 		dev_dbg(&gdev->dev, "registered chardev handle for line %d\n",
 			offset);
@@ -674,8 +674,8 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 	if (ret)
 		goto out_free_desc;
 
-	atomic_notifier_call_chain(&desc->gdev->notifier,
-				   GPIOLINE_CHANGED_REQUESTED, desc);
+	blocking_notifier_call_chain(&desc->gdev->notifier,
+				     GPIOLINE_CHANGED_REQUESTED, desc);
 
 	le->irq = gpiod_to_irq(desc);
 	if (le->irq <= 0) {
@@ -1049,8 +1049,8 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
 	priv->gdev = gdev;
 
 	priv->lineinfo_changed_nb.notifier_call = lineinfo_changed_notify;
-	ret = atomic_notifier_chain_register(&gdev->notifier,
-					     &priv->lineinfo_changed_nb);
+	ret = blocking_notifier_chain_register(&gdev->notifier,
+					       &priv->lineinfo_changed_nb);
 	if (ret)
 		goto out_free_bitmap;
 
@@ -1064,8 +1064,8 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
 	return ret;
 
 out_unregister_notifier:
-	atomic_notifier_chain_unregister(&gdev->notifier,
-					 &priv->lineinfo_changed_nb);
+	blocking_notifier_chain_unregister(&gdev->notifier,
+					   &priv->lineinfo_changed_nb);
 out_free_bitmap:
 	bitmap_free(priv->watched_lines);
 out_free_priv:
@@ -1085,8 +1085,8 @@ static int gpio_chrdev_release(struct inode *inode, struct file *file)
 	struct gpio_device *gdev = priv->gdev;
 
 	bitmap_free(priv->watched_lines);
-	atomic_notifier_chain_unregister(&gdev->notifier,
-					 &priv->lineinfo_changed_nb);
+	blocking_notifier_chain_unregister(&gdev->notifier,
+					   &priv->lineinfo_changed_nb);
 	put_device(&gdev->dev);
 	kfree(priv);
 
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 4d267c69482c..80137c1b3cdc 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -615,7 +615,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
 
 	spin_unlock_irqrestore(&gpio_lock, flags);
 
-	ATOMIC_INIT_NOTIFIER_HEAD(&gdev->notifier);
+	BLOCKING_INIT_NOTIFIER_HEAD(&gdev->notifier);
 
 #ifdef CONFIG_PINCTRL
 	INIT_LIST_HEAD(&gdev->pin_ranges);
@@ -2049,8 +2049,8 @@ 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);
+	blocking_notifier_call_chain(&desc->gdev->notifier,
+				     GPIOLINE_CHANGED_RELEASED, desc);
 
 	return ret;
 }
@@ -3927,8 +3927,8 @@ struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
 		return ERR_PTR(ret);
 	}
 
-	atomic_notifier_call_chain(&desc->gdev->notifier,
-				   GPIOLINE_CHANGED_REQUESTED, desc);
+	blocking_notifier_call_chain(&desc->gdev->notifier,
+				     GPIOLINE_CHANGED_REQUESTED, desc);
 
 	return desc;
 }
@@ -3995,8 +3995,8 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode,
 		return ERR_PTR(ret);
 	}
 
-	atomic_notifier_call_chain(&desc->gdev->notifier,
-				   GPIOLINE_CHANGED_REQUESTED, desc);
+	blocking_notifier_call_chain(&desc->gdev->notifier,
+				     GPIOLINE_CHANGED_REQUESTED, desc);
 
 	return desc;
 }
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 2dee4e1e12dc..6709f79c02dd 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -56,7 +56,7 @@ struct gpio_device {
 	const char		*label;
 	void			*data;
 	struct list_head        list;
-	struct atomic_notifier_head notifier;
+	struct blocking_notifier_head notifier;
 
 #ifdef CONFIG_PINCTRL
 	/*
-- 
2.27.0

[PATCH 09/17] gpiolib: cdev: rename priv to cdev

[Kent Gibson]

Rename priv to cdev to improve readability.

The name "priv" indicates that the object is pointed to by
file->private_data, not what the object is actually is.
As it is always used to point to a struct gpio_chardev_data, renaming
it to cdev is more appropriate.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-cdev.c | 90 ++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 352d815bbd07..fe1b385deecc 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -826,8 +826,8 @@ struct gpio_chardev_data {
  */
 static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
-	struct gpio_chardev_data *priv = file->private_data;
-	struct gpio_device *gdev = priv->gdev;
+	struct gpio_chardev_data *cdev = file->private_data;
+	struct gpio_device *gdev = cdev->gdev;
 	struct gpio_chip *gc = gdev->chip;
 	void __user *ip = (void __user *)arg;
 	struct gpio_desc *desc;
@@ -887,7 +887,7 @@ static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 		hwgpio = gpio_chip_hwgpio(desc);
 
-		if (test_bit(hwgpio, priv->watched_lines))
+		if (test_bit(hwgpio, cdev->watched_lines))
 			return -EBUSY;
 
 		gpio_desc_to_lineinfo(desc, &lineinfo);
@@ -895,7 +895,7 @@ static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		if (copy_to_user(ip, &lineinfo, sizeof(lineinfo)))
 			return -EFAULT;
 
-		set_bit(hwgpio, priv->watched_lines);
+		set_bit(hwgpio, cdev->watched_lines);
 		return 0;
 	} else if (cmd == GPIO_GET_LINEINFO_UNWATCH_IOCTL) {
 		if (copy_from_user(&offset, ip, sizeof(offset)))
@@ -907,10 +907,10 @@ static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 		hwgpio = gpio_chip_hwgpio(desc);
 
-		if (!test_bit(hwgpio, priv->watched_lines))
+		if (!test_bit(hwgpio, cdev->watched_lines))
 			return -EBUSY;
 
-		clear_bit(hwgpio, priv->watched_lines);
+		clear_bit(hwgpio, cdev->watched_lines);
 		return 0;
 	}
 	return -EINVAL;
@@ -933,12 +933,12 @@ to_gpio_chardev_data(struct notifier_block *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 gpio_chardev_data *cdev = to_gpio_chardev_data(nb);
 	struct gpioline_info_changed chg;
 	struct gpio_desc *desc = data;
 	int ret;
 
-	if (!test_bit(gpio_chip_hwgpio(desc), priv->watched_lines))
+	if (!test_bit(gpio_chip_hwgpio(desc), cdev->watched_lines))
 		return NOTIFY_DONE;
 
 	memset(&chg, 0, sizeof(chg));
@@ -947,9 +947,9 @@ static int lineinfo_changed_notify(struct notifier_block *nb,
 	chg.timestamp = ktime_get_ns();
 	gpio_desc_to_lineinfo(desc, &chg.info);
 
-	ret = kfifo_in_spinlocked(&priv->events, &chg, 1, &priv->wait.lock);
+	ret = kfifo_in_spinlocked(&cdev->events, &chg, 1, &cdev->wait.lock);
 	if (ret)
-		wake_up_poll(&priv->wait, EPOLLIN);
+		wake_up_poll(&cdev->wait, EPOLLIN);
 	else
 		pr_debug_ratelimited("lineinfo event FIFO is full - event dropped\n");
 
@@ -959,13 +959,13 @@ static int lineinfo_changed_notify(struct notifier_block *nb,
 static __poll_t lineinfo_watch_poll(struct file *file,
 				    struct poll_table_struct *pollt)
 {
-	struct gpio_chardev_data *priv = file->private_data;
+	struct gpio_chardev_data *cdev = file->private_data;
 	__poll_t events = 0;
 
-	poll_wait(file, &priv->wait, pollt);
+	poll_wait(file, &cdev->wait, pollt);
 
-	if (!kfifo_is_empty_spinlocked_noirqsave(&priv->events,
-						 &priv->wait.lock))
+	if (!kfifo_is_empty_spinlocked_noirqsave(&cdev->events,
+						 &cdev->wait.lock))
 		events = EPOLLIN | EPOLLRDNORM;
 
 	return events;
@@ -974,7 +974,7 @@ static __poll_t lineinfo_watch_poll(struct file *file,
 static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
 				   size_t count, loff_t *off)
 {
-	struct gpio_chardev_data *priv = file->private_data;
+	struct gpio_chardev_data *cdev = file->private_data;
 	struct gpioline_info_changed event;
 	ssize_t bytes_read = 0;
 	int ret;
@@ -983,28 +983,28 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
 		return -EINVAL;
 
 	do {
-		spin_lock(&priv->wait.lock);
-		if (kfifo_is_empty(&priv->events)) {
+		spin_lock(&cdev->wait.lock);
+		if (kfifo_is_empty(&cdev->events)) {
 			if (bytes_read) {
-				spin_unlock(&priv->wait.lock);
+				spin_unlock(&cdev->wait.lock);
 				return bytes_read;
 			}
 
 			if (file->f_flags & O_NONBLOCK) {
-				spin_unlock(&priv->wait.lock);
+				spin_unlock(&cdev->wait.lock);
 				return -EAGAIN;
 			}
 
-			ret = wait_event_interruptible_locked(priv->wait,
-					!kfifo_is_empty(&priv->events));
+			ret = wait_event_interruptible_locked(cdev->wait,
+					!kfifo_is_empty(&cdev->events));
 			if (ret) {
-				spin_unlock(&priv->wait.lock);
+				spin_unlock(&cdev->wait.lock);
 				return ret;
 			}
 		}
 
-		ret = kfifo_out(&priv->events, &event, 1);
-		spin_unlock(&priv->wait.lock);
+		ret = kfifo_out(&cdev->events, &event, 1);
+		spin_unlock(&cdev->wait.lock);
 		if (ret != 1) {
 			ret = -EIO;
 			break;
@@ -1029,33 +1029,33 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
 {
 	struct gpio_device *gdev = container_of(inode->i_cdev,
 						struct gpio_device, chrdev);
-	struct gpio_chardev_data *priv;
+	struct gpio_chardev_data *cdev;
 	int ret = -ENOMEM;
 
 	/* Fail on open if the backing gpiochip is gone */
 	if (!gdev->chip)
 		return -ENODEV;
 
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv)
+	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
+	if (!cdev)
 		return -ENOMEM;
 
-	priv->watched_lines = bitmap_zalloc(gdev->chip->ngpio, GFP_KERNEL);
-	if (!priv->watched_lines)
-		goto out_free_priv;
+	cdev->watched_lines = bitmap_zalloc(gdev->chip->ngpio, GFP_KERNEL);
+	if (!cdev->watched_lines)
+		goto out_free_cdev;
 
-	init_waitqueue_head(&priv->wait);
-	INIT_KFIFO(priv->events);
-	priv->gdev = gdev;
+	init_waitqueue_head(&cdev->wait);
+	INIT_KFIFO(cdev->events);
+	cdev->gdev = gdev;
 
-	priv->lineinfo_changed_nb.notifier_call = lineinfo_changed_notify;
+	cdev->lineinfo_changed_nb.notifier_call = lineinfo_changed_notify;
 	ret = blocking_notifier_chain_register(&gdev->notifier,
-					       &priv->lineinfo_changed_nb);
+					       &cdev->lineinfo_changed_nb);
 	if (ret)
 		goto out_free_bitmap;
 
 	get_device(&gdev->dev);
-	file->private_data = priv;
+	file->private_data = cdev;
 
 	ret = nonseekable_open(inode, file);
 	if (ret)
@@ -1065,11 +1065,11 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
 
 out_unregister_notifier:
 	blocking_notifier_chain_unregister(&gdev->notifier,
-					   &priv->lineinfo_changed_nb);
+					   &cdev->lineinfo_changed_nb);
 out_free_bitmap:
-	bitmap_free(priv->watched_lines);
-out_free_priv:
-	kfree(priv);
+	bitmap_free(cdev->watched_lines);
+out_free_cdev:
+	kfree(cdev);
 	return ret;
 }
 
@@ -1081,14 +1081,14 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
  */
 static int gpio_chrdev_release(struct inode *inode, struct file *file)
 {
-	struct gpio_chardev_data *priv = file->private_data;
-	struct gpio_device *gdev = priv->gdev;
+	struct gpio_chardev_data *cdev = file->private_data;
+	struct gpio_device *gdev = cdev->gdev;
 
-	bitmap_free(priv->watched_lines);
+	bitmap_free(cdev->watched_lines);
 	blocking_notifier_chain_unregister(&gdev->notifier,
-					   &priv->lineinfo_changed_nb);
+					   &cdev->lineinfo_changed_nb);
 	put_device(&gdev->dev);
-	kfree(priv);
+	kfree(cdev);
 
 	return 0;
 }
-- 
2.27.0

[PATCH 10/17] gpiolib: cdev: fix minor race in GET_LINEINFO_WATCH

[Kent Gibson]

Merge separate usage of test_bit/set_bit into test_and_set_bit to remove
the possibility of a race between the test and set.

Similarly test_bit and clear_bit.

In the existing code it is possible for two threads to race past the
test_bit and then set or clear the watch bit, and neither return EBUSY.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-cdev.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index fe1b385deecc..b2b26dc25051 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -887,15 +887,16 @@ static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 		hwgpio = gpio_chip_hwgpio(desc);
 
-		if (test_bit(hwgpio, cdev->watched_lines))
+		if (test_and_set_bit(hwgpio, cdev->watched_lines))
 			return -EBUSY;
 
 		gpio_desc_to_lineinfo(desc, &lineinfo);
 
-		if (copy_to_user(ip, &lineinfo, sizeof(lineinfo)))
+		if (copy_to_user(ip, &lineinfo, sizeof(lineinfo))) {
+			clear_bit(hwgpio, cdev->watched_lines);
 			return -EFAULT;
+		}
 
-		set_bit(hwgpio, cdev->watched_lines);
 		return 0;
 	} else if (cmd == GPIO_GET_LINEINFO_UNWATCH_IOCTL) {
 		if (copy_from_user(&offset, ip, sizeof(offset)))
@@ -907,10 +908,9 @@ static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 		hwgpio = gpio_chip_hwgpio(desc);
 
-		if (!test_bit(hwgpio, cdev->watched_lines))
+		if (!test_and_clear_bit(hwgpio, cdev->watched_lines))
 			return -EBUSY;
 
-		clear_bit(hwgpio, cdev->watched_lines);
 		return 0;
 	}
 	return -EINVAL;
-- 
2.27.0

[PATCH 11/17] gpiolib: cdev: remove recalculation of offset

[Kent Gibson]

Remove recalculation of offset from desc, where desc itself was calculated
from offset.

There is no benefit from the desc -> hwgpio conversion in this context.
The only implicit benefit of the offset -> desc -> hwgpio is
the range check in the offset -> desc, but where desc is required you
still get that, and where desc isn't required it is simpler to perform
the range check directly.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-cdev.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index b2b26dc25051..c86fb9305681 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -832,7 +832,6 @@ static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	void __user *ip = (void __user *)arg;
 	struct gpio_desc *desc;
 	__u32 offset;
-	int hwgpio;
 
 	/* We fail any subsequent ioctl():s when the chip is gone */
 	if (!gc)
@@ -860,12 +859,11 @@ static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		if (copy_from_user(&lineinfo, ip, sizeof(lineinfo)))
 			return -EFAULT;
 
+		/* this doubles as a range check on line_offset */
 		desc = gpiochip_get_desc(gc, lineinfo.line_offset);
 		if (IS_ERR(desc))
 			return PTR_ERR(desc);
 
-		hwgpio = gpio_chip_hwgpio(desc);
-
 		gpio_desc_to_lineinfo(desc, &lineinfo);
 
 		if (copy_to_user(ip, &lineinfo, sizeof(lineinfo)))
@@ -881,19 +879,18 @@ static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		if (copy_from_user(&lineinfo, ip, sizeof(lineinfo)))
 			return -EFAULT;
 
+		/* this doubles as a range check on line_offset */
 		desc = gpiochip_get_desc(gc, lineinfo.line_offset);
 		if (IS_ERR(desc))
 			return PTR_ERR(desc);
 
-		hwgpio = gpio_chip_hwgpio(desc);
-
-		if (test_and_set_bit(hwgpio, cdev->watched_lines))
+		if (test_and_set_bit(lineinfo.line_offset, cdev->watched_lines))
 			return -EBUSY;
 
 		gpio_desc_to_lineinfo(desc, &lineinfo);
 
 		if (copy_to_user(ip, &lineinfo, sizeof(lineinfo))) {
-			clear_bit(hwgpio, cdev->watched_lines);
+			clear_bit(lineinfo.line_offset, cdev->watched_lines);
 			return -EFAULT;
 		}
 
@@ -902,13 +899,10 @@ static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		if (copy_from_user(&offset, ip, sizeof(offset)))
 			return -EFAULT;
 
-		desc = gpiochip_get_desc(gc, offset);
-		if (IS_ERR(desc))
-			return PTR_ERR(desc);
-
-		hwgpio = gpio_chip_hwgpio(desc);
+		if (offset >= cdev->gdev->ngpio)
+			return -EINVAL;
 
-		if (!test_and_clear_bit(hwgpio, cdev->watched_lines))
+		if (!test_and_clear_bit(offset, cdev->watched_lines))
 			return -EBUSY;
 
 		return 0;
-- 
2.27.0

[PATCH 12/17] gpiolib: cdev: refactor linehandle cleanup into linehandle_free

[Kent Gibson]

Consolidate the cleanup of linehandles, currently duplicated in
linehandle_create and linehandle_release, into a helper function
linehandle_free.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-cdev.c | 39 ++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index c86fb9305681..d56b367239cc 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -228,17 +228,21 @@ static long linehandle_ioctl_compat(struct file *file, unsigned int cmd,
 }
 #endif
 
-static int linehandle_release(struct inode *inode, struct file *file)
+static void linehandle_free(struct linehandle_state *lh)
 {
-	struct linehandle_state *lh = file->private_data;
-	struct gpio_device *gdev = lh->gdev;
 	int i;
 
 	for (i = 0; i < lh->num_descs; i++)
-		gpiod_free(lh->descs[i]);
+		if (lh->descs[i])
+			gpiod_free(lh->descs[i]);
 	kfree(lh->label);
+	put_device(&lh->gdev->dev);
 	kfree(lh);
-	put_device(&gdev->dev);
+}
+
+static int linehandle_release(struct inode *inode, struct file *file)
+{
+	linehandle_free(file->private_data);
 	return 0;
 }
 
@@ -257,7 +261,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 	struct gpiohandle_request handlereq;
 	struct linehandle_state *lh;
 	struct file *file;
-	int fd, i, count = 0, ret;
+	int fd, i, ret;
 	u32 lflags;
 
 	if (copy_from_user(&handlereq, ip, sizeof(handlereq)))
@@ -288,6 +292,8 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 		}
 	}
 
+	lh->num_descs = handlereq.lines;
+
 	/* Request each GPIO */
 	for (i = 0; i < handlereq.lines; i++) {
 		u32 offset = handlereq.lineoffsets[i];
@@ -295,19 +301,18 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 
 		if (IS_ERR(desc)) {
 			ret = PTR_ERR(desc);
-			goto out_free_descs;
+			goto out_free_lh;
 		}
 
 		ret = gpiod_request(desc, lh->label);
 		if (ret)
-			goto out_free_descs;
+			goto out_free_lh;
 		lh->descs[i] = desc;
-		count = i + 1;
 		linehandle_flags_to_desc_flags(handlereq.flags, &desc->flags);
 
 		ret = gpiod_set_transitory(desc, false);
 		if (ret < 0)
-			goto out_free_descs;
+			goto out_free_lh;
 
 		/*
 		 * Lines have to be requested explicitly for input
@@ -318,11 +323,11 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 
 			ret = gpiod_direction_output(desc, val);
 			if (ret)
-				goto out_free_descs;
+				goto out_free_lh;
 		} else if (lflags & GPIOHANDLE_REQUEST_INPUT) {
 			ret = gpiod_direction_input(desc);
 			if (ret)
-				goto out_free_descs;
+				goto out_free_lh;
 		}
 
 		blocking_notifier_call_chain(&desc->gdev->notifier,
@@ -331,12 +336,11 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 		dev_dbg(&gdev->dev, "registered chardev handle for line %d\n",
 			offset);
 	}
-	lh->num_descs = handlereq.lines;
 
 	fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
 	if (fd < 0) {
 		ret = fd;
-		goto out_free_descs;
+		goto out_free_lh;
 	}
 
 	file = anon_inode_getfile("gpio-linehandle",
@@ -368,13 +372,8 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 
 out_put_unused_fd:
 	put_unused_fd(fd);
-out_free_descs:
-	for (i = 0; i < count; i++)
-		gpiod_free(lh->descs[i]);
-	kfree(lh->label);
 out_free_lh:
-	kfree(lh);
-	put_device(&gdev->dev);
+	linehandle_free(lh);
 	return ret;
 }
 
-- 
2.27.0

[PATCH 13/17] gpiolib: cdev: refactor lineevent cleanup into lineevent_free

[Kent Gibson]

Consolidate the cleanup of lineevents, currently duplicated in
lineevent_create and lineevent_release, into a helper function
lineevent_free.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 drivers/gpio/gpiolib-cdev.c | 44 ++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index d56b367239cc..e6c9b78adfc2 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -478,16 +478,20 @@ static ssize_t lineevent_read(struct file *file,
 	return bytes_read;
 }
 
-static int lineevent_release(struct inode *inode, struct file *file)
+static void lineevent_free(struct lineevent_state *le)
 {
-	struct lineevent_state *le = file->private_data;
-	struct gpio_device *gdev = le->gdev;
-
-	free_irq(le->irq, le);
-	gpiod_free(le->desc);
+	if (le->irq)
+		free_irq(le->irq, le);
+	if (le->desc)
+		gpiod_free(le->desc);
 	kfree(le->label);
+	put_device(&le->gdev->dev);
 	kfree(le);
-	put_device(&gdev->dev);
+}
+
+static int lineevent_release(struct inode *inode, struct file *file)
+{
+	lineevent_free(file->private_data);
 	return 0;
 }
 
@@ -612,7 +616,7 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 	u32 eflags;
 	int fd;
 	int ret;
-	int irqflags = 0;
+	int irq, irqflags = 0;
 
 	if (copy_from_user(&eventreq, ip, sizeof(eventreq)))
 		return -EFAULT;
@@ -663,7 +667,7 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 
 	ret = gpiod_request(desc, le->label);
 	if (ret)
-		goto out_free_label;
+		goto out_free_le;
 	le->desc = desc;
 	le->eflags = eflags;
 
@@ -671,16 +675,17 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 
 	ret = gpiod_direction_input(desc);
 	if (ret)
-		goto out_free_desc;
+		goto out_free_le;
 
 	blocking_notifier_call_chain(&desc->gdev->notifier,
 				     GPIOLINE_CHANGED_REQUESTED, desc);
 
-	le->irq = gpiod_to_irq(desc);
-	if (le->irq <= 0) {
+	irq = gpiod_to_irq(desc);
+	if (irq <= 0) {
 		ret = -ENODEV;
-		goto out_free_desc;
+		goto out_free_le;
 	}
+	le->irq = irq;
 
 	if (eflags & GPIOEVENT_REQUEST_RISING_EDGE)
 		irqflags |= test_bit(FLAG_ACTIVE_LOW, &desc->flags) ?
@@ -701,12 +706,12 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 				   le->label,
 				   le);
 	if (ret)
-		goto out_free_desc;
+		goto out_free_le;
 
 	fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
 	if (fd < 0) {
 		ret = fd;
-		goto out_free_irq;
+		goto out_free_le;
 	}
 
 	file = anon_inode_getfile("gpio-event",
@@ -735,15 +740,8 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 
 out_put_unused_fd:
 	put_unused_fd(fd);
-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);
+	lineevent_free(le);
 	return ret;
 }
 
-- 
2.27.0

[PATCH 14/17] gpio: uapi: fix misplaced comment line

[Kent Gibson]

The second line of the description for event_type is before the first.
Move it to after the first line.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 include/uapi/linux/gpio.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index 0206383c0383..9c27cecf406f 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -71,8 +71,8 @@ enum {
  * 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
+ * and GPIOLINE_CHANGED_CONFIG
  *
  * 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
-- 
2.27.0

[PATCH 15/17] tools: gpio: fix spurious close warning in lsgpio

[Kent Gibson]

Fix bogus close warning that occurs when opening the character device
fails.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 tools/gpio/lsgpio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/gpio/lsgpio.c b/tools/gpio/lsgpio.c
index 8a71ad36f83b..b08d7a5e779b 100644
--- a/tools/gpio/lsgpio.c
+++ b/tools/gpio/lsgpio.c
@@ -94,7 +94,7 @@ int list_device(const char *device_name)
 	if (fd == -1) {
 		ret = -errno;
 		fprintf(stderr, "Failed to open %s\n", chrdev_name);
-		goto exit_close_error;
+		goto exit_free_name;
 	}
 
 	/* Inspect this GPIO chip */
@@ -141,6 +141,7 @@ int list_device(const char *device_name)
 exit_close_error:
 	if (close(fd) == -1)
 		perror("Failed to close GPIO character device file");
+exit_free_name:
 	free(chrdev_name);
 	return ret;
 }
-- 
2.27.0

[PATCH 16/17] tools: gpio: fix spurious close warning in gpio-utils

[Kent Gibson]

Fix bogus close warning that occurs when opening the character device
fails.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 tools/gpio/gpio-utils.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/gpio/gpio-utils.c b/tools/gpio/gpio-utils.c
index 06003789e7c7..16a5d9cb9da2 100644
--- a/tools/gpio/gpio-utils.c
+++ b/tools/gpio/gpio-utils.c
@@ -75,7 +75,7 @@ int gpiotools_request_linehandle(const char *device_name, unsigned int *lines,
 		ret = -errno;
 		fprintf(stderr, "Failed to open %s, %s\n",
 			chrdev_name, strerror(errno));
-		goto exit_close_error;
+		goto exit_free_name;
 	}
 
 	for (i = 0; i < nlines; i++)
@@ -94,9 +94,9 @@ int gpiotools_request_linehandle(const char *device_name, unsigned int *lines,
 			"GPIO_GET_LINEHANDLE_IOCTL", ret, strerror(errno));
 	}
 
-exit_close_error:
 	if (close(fd) == -1)
 		perror("Failed to close GPIO character device file");
+exit_free_name:
 	free(chrdev_name);
 	return ret < 0 ? ret : req.fd;
 }
-- 
2.27.0

[PATCH 17/17] tools: gpio: fix spurious close warning in gpio-event-mon

[Kent Gibson]

Fix bogus close warning that occurs when opening the character device
fails.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 tools/gpio/gpio-event-mon.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/gpio/gpio-event-mon.c b/tools/gpio/gpio-event-mon.c
index 30ed0e06f52a..1a303a81aeef 100644
--- a/tools/gpio/gpio-event-mon.c
+++ b/tools/gpio/gpio-event-mon.c
@@ -45,7 +45,7 @@ int monitor_device(const char *device_name,
 	if (fd == -1) {
 		ret = -errno;
 		fprintf(stderr, "Failed to open %s\n", chrdev_name);
-		goto exit_close_error;
+		goto exit_free_name;
 	}
 
 	req.lineoffset = line;
@@ -117,6 +117,7 @@ int monitor_device(const char *device_name,
 exit_close_error:
 	if (close(fd) == -1)
 		perror("Failed to close GPIO character device file");
+exit_free_name:
 	free(chrdev_name);
 	return ret;
 }
-- 
2.27.0

Re: [PATCH 08/17] gpiolib: cdev: use blocking notifier call chain instead of atomic

[Bartosz Golaszewski]

On Wed, Jul 8, 2020 at 6:19 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> Replace usage of atomic_notifier_call_chain with
> blocking_notifier_call_chain as the notifier function,
> lineinfo_changed_notify, calls gpio_desc_to_lineinfo,
> which calls pinctrl_gpio_can_use_line, which can sleep.
>
> The chain isn't being called from an atomic context so the
> the blocking notifier is a suitable substitute.
>
> Signed-off-by: Kent Gibson <warthog618@gmail.com>

In the back of my mind I still think I chose atomic notifiers for a
reason but I can't remember it now. Anyway, we can apply it and see if
anything bad happens and potentially just revert it.

Bart

Re: [PATCH 00/17] gpiolib: cdev: pre-uAPI v2 cleanups

[Bartosz Golaszewski]

On Wed, Jul 8, 2020 at 6:18 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> This collection of patches provides improvements to or
> address minor problems in gpiolib-cdev.
>
> The majority of the patches (1-7, 9-11) have been pulled directly from
> my "gpio: cdev: add uAPI V2" patch set, as they are not related to any
> uAPI changes.
> The remaining patches were either split out of the remaining patches
> from that set, as they are not directly part of the uAPI changes, or
> were inspired by fixes for issues in that set, or were only noticed
> subsequent to that set.
>
> Changes since "gpio: cdev: add uAPI V2":
>  - rebase onto latest gpio/devel
>  - fix typo in patch 1 commit description
>  - replace patch 8 with the blocking notifier call chain patch
>  - rename priv to cdev instead of gcdev in patch 9
>  - fix error handling in patch 10
>  - add patches 12 to 17
>
> Kent Gibson (17):
>   gpiolib: move gpiolib-sysfs function declarations into their own
>     header
>   gpiolib: cdev: sort includes
>   gpiolib: cdev: minor indentation fixes
>   gpiolib: cdev: refactor gpiohandle_flags_to_desc_flags
>   gpiolib: cdev: rename 'filep' and 'filp' to 'file' to be consistent
>     with other use
>   gpiolib: cdev: rename numdescs to num_descs
>   gpiolib: cdev: remove pointless decrement of i
>   gpiolib: cdev: use blocking notifier call chain instead of atomic
>   gpiolib: cdev: rename priv to cdev
>   gpiolib: cdev: fix minor race in GET_LINEINFO_WATCH
>   gpiolib: cdev: remove recalculation of offset
>   gpiolib: cdev: refactor linehandle cleanup into linehandle_free
>   gpiolib: cdev: refactor lineevent cleanup into lineevent_free
>   gpio: uapi: fix misplaced comment line
>   tools: gpio: fix spurious close warning in lsgpio
>   tools: gpio: fix spurious close warning in gpio-utils
>   tools: gpio: fix spurious close warning in gpio-event-mon
>
>  drivers/gpio/gpiolib-cdev.c  | 385 ++++++++++++++++-------------------
>  drivers/gpio/gpiolib-sysfs.c |   1 +
>  drivers/gpio/gpiolib-sysfs.h |  24 +++
>  drivers/gpio/gpiolib.c       |  15 +-
>  drivers/gpio/gpiolib.h       |  20 +-
>  include/uapi/linux/gpio.h    |   2 +-
>  tools/gpio/gpio-event-mon.c  |   3 +-
>  tools/gpio/gpio-utils.c      |   4 +-
>  tools/gpio/lsgpio.c          |   3 +-
>  9 files changed, 217 insertions(+), 240 deletions(-)
>  create mode 100644 drivers/gpio/gpiolib-sysfs.h
>
>
> base-commit: b239e4454e59bc85d466eb5630da46f6a876df77
> --
> 2.27.0
>

Hi Kent,

The entire series looks good to me, thanks for doing this. I'll pick
it up into my tree and send a PR to Linus.

Bartosz