diff mbox series

[v2,5/5] gpiolib: cdev: replace locking wrappers for gpio_device with guards

Message ID 20231221012040.17763-6-warthog618@gmail.com
State Accepted
Commit 20bddcb40b2b6d6028fb4224fab4aadb7d7b2674
Headers show
Series gpiolib: cdev: guard tidying | expand

Commit Message

Kent Gibson Dec. 21, 2023, 1:20 a.m. UTC 
Replace the wrapping functions that inhibit removal of the gpio chip
with equivalent guards.

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

Comments

Kent Gibson Dec. 22, 2023, 12:47 a.m. UTC | #1 
On Thu, Dec 21, 2023 at 05:48:52PM +0200, Andy Shevchenko wrote:
> On Thu, Dec 21, 2023 at 09:20:40AM +0800, Kent Gibson wrote:
> > Replace the wrapping functions that inhibit removal of the gpio chip
>
> GPIO
>

Bart, I don't care either way and not enough to respin a v3.
If it bothers you could you fix it on the way in?

That is if you aren't too busy reversing xmas trees ;-).

Thanks,
Kent.
Bartosz Golaszewski Dec. 22, 2023, 2:05 p.m. UTC | #2 
On Fri, Dec 22, 2023 at 1:47 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Thu, Dec 21, 2023 at 05:48:52PM +0200, Andy Shevchenko wrote:
> > On Thu, Dec 21, 2023 at 09:20:40AM +0800, Kent Gibson wrote:
> > > Replace the wrapping functions that inhibit removal of the gpio chip
> >
> > GPIO
> >
>
> Bart, I don't care either way and not enough to respin a v3.
> If it bothers you could you fix it on the way in?
>

Sure!

> That is if you aren't too busy reversing xmas trees ;-).
>

Joke's on you, I actually do find them easier to read and try to use
them everywhere in new code I write. :)

Bart

> Thanks,
> Kent.
>
Kent Gibson Dec. 22, 2023, 2:12 p.m. UTC | #3 
On Fri, Dec 22, 2023 at 03:05:41PM +0100, Bartosz Golaszewski wrote:
> On Fri, Dec 22, 2023 at 1:47 AM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Thu, Dec 21, 2023 at 05:48:52PM +0200, Andy Shevchenko wrote:
> > > On Thu, Dec 21, 2023 at 09:20:40AM +0800, Kent Gibson wrote:
> > > > Replace the wrapping functions that inhibit removal of the gpio chip
> > >
> > > GPIO
> > >
> >
> > Bart, I don't care either way and not enough to respin a v3.
> > If it bothers you could you fix it on the way in?
> >
>
> Sure!
>
> > That is if you aren't too busy reversing xmas trees ;-).
> >
>
> Joke's on you, I actually do find them easier to read and try to use
> them everywhere in new code I write. :)
>

Oh, I agree - it is more readable.  It just seems very timely that Andy
keeps griping about them not being inverted.

Cheers,
Kent.
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 9ff2b447cc20..2a88736629ef 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -24,6 +24,7 @@ 
 #include <linux/pinctrl/consumer.h>
 #include <linux/poll.h>
 #include <linux/rbtree.h>
+#include <linux/rwsem.h>
 #include <linux/seq_file.h>
 #include <linux/spinlock.h>
 #include <linux/timekeeping.h>
@@ -65,45 +66,6 @@  typedef long (*ioctl_fn)(struct file *, unsigned int, unsigned long);
 typedef ssize_t (*read_fn)(struct file *, char __user *,
 			   size_t count, loff_t *);
 
-static __poll_t call_poll_locked(struct file *file,
-				 struct poll_table_struct *wait,
-				 struct gpio_device *gdev, poll_fn func)
-{
-	__poll_t ret;
-
-	down_read(&gdev->sem);
-	ret = func(file, wait);
-	up_read(&gdev->sem);
-
-	return ret;
-}
-
-static long call_ioctl_locked(struct file *file, unsigned int cmd,
-			      unsigned long arg, struct gpio_device *gdev,
-			      ioctl_fn func)
-{
-	long ret;
-
-	down_read(&gdev->sem);
-	ret = func(file, cmd, arg);
-	up_read(&gdev->sem);
-
-	return ret;
-}
-
-static ssize_t call_read_locked(struct file *file, char __user *buf,
-				size_t count, loff_t *f_ps,
-				struct gpio_device *gdev, read_fn func)
-{
-	ssize_t ret;
-
-	down_read(&gdev->sem);
-	ret = func(file, buf, count, f_ps);
-	up_read(&gdev->sem);
-
-	return ret;
-}
-
 /*
  * GPIO line handle management
  */
@@ -238,8 +200,8 @@  static long linehandle_set_config(struct linehandle_state *lh,
 	return 0;
 }
 
-static long linehandle_ioctl_unlocked(struct file *file, unsigned int cmd,
-				      unsigned long arg)
+static long linehandle_ioctl(struct file *file, unsigned int cmd,
+			     unsigned long arg)
 {
 	struct linehandle_state *lh = file->private_data;
 	void __user *ip = (void __user *)arg;
@@ -248,6 +210,8 @@  static long linehandle_ioctl_unlocked(struct file *file, unsigned int cmd,
 	unsigned int i;
 	int ret;
 
+	guard(rwsem_read)(&lh->gdev->sem);
+
 	if (!lh->gdev->chip)
 		return -ENODEV;
 
@@ -297,15 +261,6 @@  static long linehandle_ioctl_unlocked(struct file *file, unsigned int cmd,
 	}
 }
 
-static long linehandle_ioctl(struct file *file, unsigned int cmd,
-			     unsigned long arg)
-{
-	struct linehandle_state *lh = file->private_data;
-
-	return call_ioctl_locked(file, cmd, arg, lh->gdev,
-				 linehandle_ioctl_unlocked);
-}
-
 #ifdef CONFIG_COMPAT
 static long linehandle_ioctl_compat(struct file *file, unsigned int cmd,
 				    unsigned long arg)
@@ -1564,12 +1519,14 @@  static long linereq_set_config(struct linereq *lr, void __user *ip)
 	return 0;
 }
 
-static long linereq_ioctl_unlocked(struct file *file, unsigned int cmd,
-				   unsigned long arg)
+static long linereq_ioctl(struct file *file, unsigned int cmd,
+			  unsigned long arg)
 {
 	struct linereq *lr = file->private_data;
 	void __user *ip = (void __user *)arg;
 
+	guard(rwsem_read)(&lr->gdev->sem);
+
 	if (!lr->gdev->chip)
 		return -ENODEV;
 
@@ -1585,15 +1542,6 @@  static long linereq_ioctl_unlocked(struct file *file, unsigned int cmd,
 	}
 }
 
-static long linereq_ioctl(struct file *file, unsigned int cmd,
-			  unsigned long arg)
-{
-	struct linereq *lr = file->private_data;
-
-	return call_ioctl_locked(file, cmd, arg, lr->gdev,
-				 linereq_ioctl_unlocked);
-}
-
 #ifdef CONFIG_COMPAT
 static long linereq_ioctl_compat(struct file *file, unsigned int cmd,
 				 unsigned long arg)
@@ -1602,12 +1550,14 @@  static long linereq_ioctl_compat(struct file *file, unsigned int cmd,
 }
 #endif
 
-static __poll_t linereq_poll_unlocked(struct file *file,
-				      struct poll_table_struct *wait)
+static __poll_t linereq_poll(struct file *file,
+			     struct poll_table_struct *wait)
 {
 	struct linereq *lr = file->private_data;
 	__poll_t events = 0;
 
+	guard(rwsem_read)(&lr->gdev->sem);
+
 	if (!lr->gdev->chip)
 		return EPOLLHUP | EPOLLERR;
 
@@ -1620,22 +1570,16 @@  static __poll_t linereq_poll_unlocked(struct file *file,
 	return events;
 }
 
-static __poll_t linereq_poll(struct file *file,
-			     struct poll_table_struct *wait)
-{
-	struct linereq *lr = file->private_data;
-
-	return call_poll_locked(file, wait, lr->gdev, linereq_poll_unlocked);
-}
-
-static ssize_t linereq_read_unlocked(struct file *file, char __user *buf,
-				     size_t count, loff_t *f_ps)
+static ssize_t linereq_read(struct file *file, char __user *buf,
+			    size_t count, loff_t *f_ps)
 {
 	struct linereq *lr = file->private_data;
 	struct gpio_v2_line_event le;
 	ssize_t bytes_read = 0;
 	int ret;
 
+	guard(rwsem_read)(&lr->gdev->sem);
+
 	if (!lr->gdev->chip)
 		return -ENODEV;
 
@@ -1677,15 +1621,6 @@  static ssize_t linereq_read_unlocked(struct file *file, char __user *buf,
 	return bytes_read;
 }
 
-static ssize_t linereq_read(struct file *file, char __user *buf,
-			    size_t count, loff_t *f_ps)
-{
-	struct linereq *lr = file->private_data;
-
-	return call_read_locked(file, buf, count, f_ps, lr->gdev,
-				linereq_read_unlocked);
-}
-
 static void linereq_free(struct linereq *lr)
 {
 	struct line *line;
@@ -1938,12 +1873,14 @@  struct lineevent_state {
 	(GPIOEVENT_REQUEST_RISING_EDGE | \
 	GPIOEVENT_REQUEST_FALLING_EDGE)
 
-static __poll_t lineevent_poll_unlocked(struct file *file,
-					struct poll_table_struct *wait)
+static __poll_t lineevent_poll(struct file *file,
+			       struct poll_table_struct *wait)
 {
 	struct lineevent_state *le = file->private_data;
 	__poll_t events = 0;
 
+	guard(rwsem_read)(&le->gdev->sem);
+
 	if (!le->gdev->chip)
 		return EPOLLHUP | EPOLLERR;
 
@@ -1955,14 +1892,6 @@  static __poll_t lineevent_poll_unlocked(struct file *file,
 	return events;
 }
 
-static __poll_t lineevent_poll(struct file *file,
-			       struct poll_table_struct *wait)
-{
-	struct lineevent_state *le = file->private_data;
-
-	return call_poll_locked(file, wait, le->gdev, lineevent_poll_unlocked);
-}
-
 static int lineevent_unregistered_notify(struct notifier_block *nb,
 					 unsigned long action, void *data)
 {
@@ -1979,8 +1908,8 @@  struct compat_gpioeevent_data {
 	u32		id;
 };
 
-static ssize_t lineevent_read_unlocked(struct file *file, char __user *buf,
-				       size_t count, loff_t *f_ps)
+static ssize_t lineevent_read(struct file *file, char __user *buf,
+			      size_t count, loff_t *f_ps)
 {
 	struct lineevent_state *le = file->private_data;
 	struct gpioevent_data ge;
@@ -1988,6 +1917,8 @@  static ssize_t lineevent_read_unlocked(struct file *file, char __user *buf,
 	ssize_t ge_size;
 	int ret;
 
+	guard(rwsem_read)(&le->gdev->sem);
+
 	if (!le->gdev->chip)
 		return -ENODEV;
 
@@ -2042,15 +1973,6 @@  static ssize_t lineevent_read_unlocked(struct file *file, char __user *buf,
 	return bytes_read;
 }
 
-static ssize_t lineevent_read(struct file *file, char __user *buf,
-			      size_t count, loff_t *f_ps)
-{
-	struct lineevent_state *le = file->private_data;
-
-	return call_read_locked(file, buf, count, f_ps, le->gdev,
-				lineevent_read_unlocked);
-}
-
 static void lineevent_free(struct lineevent_state *le)
 {
 	if (le->device_unregistered_nb.notifier_call)
@@ -2071,13 +1993,15 @@  static int lineevent_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
-static long lineevent_ioctl_unlocked(struct file *file, unsigned int cmd,
-				     unsigned long arg)
+static long lineevent_ioctl(struct file *file, unsigned int cmd,
+			    unsigned long arg)
 {
 	struct lineevent_state *le = file->private_data;
 	void __user *ip = (void __user *)arg;
 	struct gpiohandle_data ghd;
 
+	guard(rwsem_read)(&le->gdev->sem);
+
 	if (!le->gdev->chip)
 		return -ENODEV;
 
@@ -2103,15 +2027,6 @@  static long lineevent_ioctl_unlocked(struct file *file, unsigned int cmd,
 	return -EINVAL;
 }
 
-static long lineevent_ioctl(struct file *file, unsigned int cmd,
-			    unsigned long arg)
-{
-	struct lineevent_state *le = file->private_data;
-
-	return call_ioctl_locked(file, cmd, arg, le->gdev,
-				 lineevent_ioctl_unlocked);
-}
-
 #ifdef CONFIG_COMPAT
 static long lineevent_ioctl_compat(struct file *file, unsigned int cmd,
 				   unsigned long arg)
@@ -2584,12 +2499,17 @@  static int lineinfo_unwatch(struct gpio_chardev_data *cdev, void __user *ip)
 	return 0;
 }
 
-static long gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned long arg)
+/*
+ * gpio_ioctl() - ioctl handler for the GPIO chardev
+ */
+static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	struct gpio_chardev_data *cdev = file->private_data;
 	struct gpio_device *gdev = cdev->gdev;
 	void __user *ip = (void __user *)arg;
 
+	guard(rwsem_read)(&gdev->sem);
+
 	/* We fail any subsequent ioctl():s when the chip is gone */
 	if (!gdev->chip)
 		return -ENODEV;
@@ -2621,17 +2541,6 @@  static long gpio_ioctl_unlocked(struct file *file, unsigned int cmd, unsigned lo
 	}
 }
 
-/*
- * gpio_ioctl() - ioctl handler for the GPIO chardev
- */
-static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
-	struct gpio_chardev_data *cdev = file->private_data;
-
-	return call_ioctl_locked(file, cmd, arg, cdev->gdev,
-				 gpio_ioctl_unlocked);
-}
-
 #ifdef CONFIG_COMPAT
 static long gpio_ioctl_compat(struct file *file, unsigned int cmd,
 			      unsigned long arg)
@@ -2679,12 +2588,14 @@  static int gpio_device_unregistered_notify(struct notifier_block *nb,
 	return NOTIFY_OK;
 }
 
-static __poll_t lineinfo_watch_poll_unlocked(struct file *file,
-					     struct poll_table_struct *pollt)
+static __poll_t lineinfo_watch_poll(struct file *file,
+				    struct poll_table_struct *pollt)
 {
 	struct gpio_chardev_data *cdev = file->private_data;
 	__poll_t events = 0;
 
+	guard(rwsem_read)(&cdev->gdev->sem);
+
 	if (!cdev->gdev->chip)
 		return EPOLLHUP | EPOLLERR;
 
@@ -2697,17 +2608,8 @@  static __poll_t lineinfo_watch_poll_unlocked(struct file *file,
 	return events;
 }
 
-static __poll_t lineinfo_watch_poll(struct file *file,
-				    struct poll_table_struct *pollt)
-{
-	struct gpio_chardev_data *cdev = file->private_data;
-
-	return call_poll_locked(file, pollt, cdev->gdev,
-				lineinfo_watch_poll_unlocked);
-}
-
-static ssize_t lineinfo_watch_read_unlocked(struct file *file, char __user *buf,
-					    size_t count, loff_t *off)
+static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
+				   size_t count, loff_t *off)
 {
 	struct gpio_chardev_data *cdev = file->private_data;
 	struct gpio_v2_line_info_changed event;
@@ -2715,6 +2617,8 @@  static ssize_t lineinfo_watch_read_unlocked(struct file *file, char __user *buf,
 	int ret;
 	size_t event_size;
 
+	guard(rwsem_read)(&cdev->gdev->sem);
+
 	if (!cdev->gdev->chip)
 		return -ENODEV;
 
@@ -2777,15 +2681,6 @@  static ssize_t lineinfo_watch_read_unlocked(struct file *file, char __user *buf,
 	return bytes_read;
 }
 
-static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
-				   size_t count, loff_t *off)
-{
-	struct gpio_chardev_data *cdev = file->private_data;
-
-	return call_read_locked(file, buf, count, off, cdev->gdev,
-				lineinfo_watch_read_unlocked);
-}
-
 /**
  * gpio_chrdev_open() - open the chardev for ioctl operations
  * @inode: inode for this chardev
@@ -2799,17 +2694,15 @@  static int gpio_chrdev_open(struct inode *inode, struct file *file)
 	struct gpio_chardev_data *cdev;
 	int ret = -ENOMEM;
 
-	down_read(&gdev->sem);
+	guard(rwsem_read)(&gdev->sem);
 
 	/* Fail on open if the backing gpiochip is gone */
-	if (!gdev->chip) {
-		ret = -ENODEV;
-		goto out_unlock;
-	}
+	if (!gdev->chip)
+		return -ENODEV;
 
 	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
 	if (!cdev)
-		goto out_unlock;
+		return -ENODEV;
 
 	cdev->watched_lines = bitmap_zalloc(gdev->chip->ngpio, GFP_KERNEL);
 	if (!cdev->watched_lines)
@@ -2838,8 +2731,6 @@  static int gpio_chrdev_open(struct inode *inode, struct file *file)
 	if (ret)
 		goto out_unregister_device_notifier;
 
-	up_read(&gdev->sem);
-
 	return ret;
 
 out_unregister_device_notifier:
@@ -2853,8 +2744,6 @@  static int gpio_chrdev_open(struct inode *inode, struct file *file)
 	bitmap_free(cdev->watched_lines);
 out_free_cdev:
 	kfree(cdev);
-out_unlock:
-	up_read(&gdev->sem);
 	return ret;
 }