[PATCH v3 0/4] gpiolib: A fix and a few cleanups

[PATCH v3 0/4] gpiolib: A fix and a few cleanups

[Andy Shevchenko]

Patch 1 is a fix for wrong error code to user space.
Patches 2-4 are small cleanups.

Can be routed via my tree, or directly into GPIO, whatever maintainers
prefer.

Changelog v3:
- joined two patches into a single fix (Bart)
- added Rb tag (Linus)
- renamed for_each_gpio_desc_with_flag() macro to be descriptive (Johan)
- added two new cleanups

Andy Shevchenko (4):
  gpiolib: Never return internal error codes to user space
  gpiolib: Introduce for_each_gpio_desc_with_flag() macro
  gpiolib: Use short form of ternary operator in gpiod_get_index()
  gpiolib: Simplify error path in gpiod_get_index() when requesting GPIO

 drivers/gpio/gpiolib-cdev.c  |  6 +++---
 drivers/gpio/gpiolib-of.c    | 10 ++++------
 drivers/gpio/gpiolib-sysfs.c | 14 ++++----------
 drivers/gpio/gpiolib.c       | 35 ++++++++++++++++-------------------
 drivers/gpio/gpiolib.h       | 19 +++++++++++++++++++
 5 files changed, 46 insertions(+), 38 deletions(-)

-- 
2.34.1

[PATCH v3 1/4] gpiolib: Never return internal error codes to user space

[Andy Shevchenko]

Currently it's possible that character device interface may return
the error codes which are not supposed to be seen by user space.
In this case it's EPROBE_DEFER.

Wrap it to return -ENODEV instead as sysfs does.

Fixes: d7c51b47ac11 ("gpio: userspace ABI for reading/writing GPIO lines")
Fixes: 61f922db7221 ("gpio: userspace ABI for reading GPIO line events")
Fixes: 3c0d9c635ae2 ("gpiolib: cdev: support GPIO_V2_GET_LINE_IOCTL and GPIO_V2_LINE_GET_VALUES_IOCTL")
Reported-by: Suresh Balakrishnan <suresh.balakrishnan@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib-cdev.c  |  6 +++---
 drivers/gpio/gpiolib-sysfs.c |  7 ++-----
 drivers/gpio/gpiolib.h       | 12 ++++++++++++
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index c7b5446d01fd..ffa0256cad5a 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -330,7 +330,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
 			goto out_free_lh;
 		}
 
-		ret = gpiod_request(desc, lh->label);
+		ret = gpiod_request_user(desc, lh->label);
 		if (ret)
 			goto out_free_lh;
 		lh->descs[i] = desc;
@@ -1378,7 +1378,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
 			goto out_free_linereq;
 		}
 
-		ret = gpiod_request(desc, lr->label);
+		ret = gpiod_request_user(desc, lr->label);
 		if (ret)
 			goto out_free_linereq;
 
@@ -1764,7 +1764,7 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 		}
 	}
 
-	ret = gpiod_request(desc, le->label);
+	ret = gpiod_request_user(desc, le->label);
 	if (ret)
 		goto out_free_le;
 	le->desc = desc;
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 4098bc7f88b7..44c1ad51b3fe 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -475,12 +475,9 @@ static ssize_t export_store(struct class *class,
 	 * they may be undone on its behalf too.
 	 */
 
-	status = gpiod_request(desc, "sysfs");
-	if (status) {
-		if (status == -EPROBE_DEFER)
-			status = -ENODEV;
+	status = gpiod_request_user(desc, "sysfs");
+	if (status)
 		goto done;
-	}
 
 	status = gpiod_set_transitory(desc, false);
 	if (!status) {
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 30bc3f80f83e..c31f4626915d 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -135,6 +135,18 @@ struct gpio_desc {
 
 int gpiod_request(struct gpio_desc *desc, const char *label);
 void gpiod_free(struct gpio_desc *desc);
+
+static inline int gpiod_request_user(struct gpio_desc *desc, const char *label)
+{
+	int ret;
+
+	ret = gpiod_request(desc, label);
+	if (ret == -EPROBE_DEFER)
+		ret = -ENODEV;
+
+	return ret;
+}
+
 int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
 		unsigned long lflags, enum gpiod_flags dflags);
 int gpio_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce);
-- 
2.34.1

Re: [PATCH v3 1/4] gpiolib: Never return internal error codes to user space

[Bartosz Golaszewski]

On Tue, Feb 1, 2022 at 4:28 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Currently it's possible that character device interface may return
> the error codes which are not supposed to be seen by user space.
> In this case it's EPROBE_DEFER.
>
> Wrap it to return -ENODEV instead as sysfs does.
>
> Fixes: d7c51b47ac11 ("gpio: userspace ABI for reading/writing GPIO lines")
> Fixes: 61f922db7221 ("gpio: userspace ABI for reading GPIO line events")
> Fixes: 3c0d9c635ae2 ("gpiolib: cdev: support GPIO_V2_GET_LINE_IOCTL and GPIO_V2_LINE_GET_VALUES_IOCTL")
> Reported-by: Suresh Balakrishnan <suresh.balakrishnan@intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpio/gpiolib-cdev.c  |  6 +++---
>  drivers/gpio/gpiolib-sysfs.c |  7 ++-----
>  drivers/gpio/gpiolib.h       | 12 ++++++++++++
>  3 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index c7b5446d01fd..ffa0256cad5a 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -330,7 +330,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
>                         goto out_free_lh;
>                 }
>
> -               ret = gpiod_request(desc, lh->label);
> +               ret = gpiod_request_user(desc, lh->label);
>                 if (ret)
>                         goto out_free_lh;
>                 lh->descs[i] = desc;
> @@ -1378,7 +1378,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
>                         goto out_free_linereq;
>                 }
>
> -               ret = gpiod_request(desc, lr->label);
> +               ret = gpiod_request_user(desc, lr->label);
>                 if (ret)
>                         goto out_free_linereq;
>
> @@ -1764,7 +1764,7 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
>                 }
>         }
>
> -       ret = gpiod_request(desc, le->label);
> +       ret = gpiod_request_user(desc, le->label);
>         if (ret)
>                 goto out_free_le;
>         le->desc = desc;
> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
> index 4098bc7f88b7..44c1ad51b3fe 100644
> --- a/drivers/gpio/gpiolib-sysfs.c
> +++ b/drivers/gpio/gpiolib-sysfs.c
> @@ -475,12 +475,9 @@ static ssize_t export_store(struct class *class,
>          * they may be undone on its behalf too.
>          */
>
> -       status = gpiod_request(desc, "sysfs");
> -       if (status) {
> -               if (status == -EPROBE_DEFER)
> -                       status = -ENODEV;
> +       status = gpiod_request_user(desc, "sysfs");
> +       if (status)
>                 goto done;
> -       }
>
>         status = gpiod_set_transitory(desc, false);
>         if (!status) {
> diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
> index 30bc3f80f83e..c31f4626915d 100644
> --- a/drivers/gpio/gpiolib.h
> +++ b/drivers/gpio/gpiolib.h
> @@ -135,6 +135,18 @@ struct gpio_desc {
>
>  int gpiod_request(struct gpio_desc *desc, const char *label);
>  void gpiod_free(struct gpio_desc *desc);
> +
> +static inline int gpiod_request_user(struct gpio_desc *desc, const char *label)
> +{
> +       int ret;
> +
> +       ret = gpiod_request(desc, label);
> +       if (ret == -EPROBE_DEFER)
> +               ret = -ENODEV;
> +
> +       return ret;
> +}
> +
>  int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
>                 unsigned long lflags, enum gpiod_flags dflags);
>  int gpio_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce);
> --
> 2.34.1
>

Queued for fixes, thanks!

Bart

[PATCH v3 2/4] gpiolib: Introduce for_each_gpio_desc_with_flag() macro

[Andy Shevchenko]

In a few places we are using a loop against all GPIO descriptors
with a given flag for a given device. Replace it with a consolidated
for_each type of macro.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpiolib-of.c    | 10 ++++------
 drivers/gpio/gpiolib-sysfs.c |  7 ++-----
 drivers/gpio/gpiolib.c       |  7 +++----
 drivers/gpio/gpiolib.h       |  7 +++++++
 4 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 91dcf2c6cdd8..ae1ce319cd78 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -711,14 +711,12 @@ static int of_gpiochip_scan_gpios(struct gpio_chip *chip)
 static void of_gpiochip_remove_hog(struct gpio_chip *chip,
 				   struct device_node *hog)
 {
-	struct gpio_desc *descs = chip->gpiodev->descs;
+	struct gpio_desc *desc;
 	unsigned int i;
 
-	for (i = 0; i < chip->ngpio; i++) {
-		if (test_bit(FLAG_IS_HOGGED, &descs[i].flags) &&
-		    descs[i].hog == hog)
-			gpiochip_free_own_desc(&descs[i]);
-	}
+	for_each_gpio_desc_with_flag(i, chip, desc, FLAG_IS_HOGGED)
+		if (desc->hog == hog)
+			gpiochip_free_own_desc(desc);
 }
 
 static int of_gpiochip_match_node(struct gpio_chip *chip, void *data)
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 44c1ad51b3fe..78ca7dee8b64 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -790,11 +790,8 @@ void gpiochip_sysfs_unregister(struct gpio_device *gdev)
 	mutex_unlock(&sysfs_lock);
 
 	/* unregister gpiod class devices owned by sysfs */
-	for (i = 0; i < chip->ngpio; i++) {
-		desc = &gdev->descs[i];
-		if (test_and_clear_bit(FLAG_SYSFS, &desc->flags))
-			gpiod_free(desc);
-	}
+	for_each_gpio_desc_with_flag(i, chip, desc, FLAG_SYSFS)
+		gpiod_free(desc);
 }
 
 static int __init gpiolib_sysfs_init(void)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 3859911b61e9..e3702bc1b533 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4102,12 +4102,11 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
  */
 static void gpiochip_free_hogs(struct gpio_chip *gc)
 {
+	struct gpio_desc *desc;
 	int id;
 
-	for (id = 0; id < gc->ngpio; id++) {
-		if (test_bit(FLAG_IS_HOGGED, &gc->gpiodev->descs[id].flags))
-			gpiochip_free_own_desc(&gc->gpiodev->descs[id]);
-	}
+	for_each_gpio_desc_with_flag(id, gc, desc, FLAG_IS_HOGGED)
+		gpiochip_free_own_desc(desc);
 }
 
 /**
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index c31f4626915d..dbbad15607e3 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -82,6 +82,13 @@ struct gpio_array {
 };
 
 struct gpio_desc *gpiochip_get_desc(struct gpio_chip *gc, unsigned int hwnum);
+
+#define for_each_gpio_desc_with_flag(i, gc, desc, flag)		\
+	for (i = 0, desc = gpiochip_get_desc(gc, i);		\
+	     i < gc->ngpio;					\
+	     i++, desc = gpiochip_get_desc(gc, i))		\
+		if (!test_bit(flag, &desc->flags)) {} else
+
 int gpiod_get_array_value_complex(bool raw, bool can_sleep,
 				  unsigned int array_size,
 				  struct gpio_desc **desc_array,
-- 
2.34.1

[PATCH v3 3/4] gpiolib: Use short form of ternary operator in gpiod_get_index()

[Andy Shevchenko]

Instead of repeating first argument for true branch, use short
form of the ternary operator, i.e. ?:.

While at it, fix a typo in the comment.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e3702bc1b533..daedf8207173 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3931,19 +3931,18 @@ struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
 	 * If a connection label was passed use that, else attempt to use
 	 * the device name as label
 	 */
-	ret = gpiod_request(desc, con_id ? con_id : devname);
+	ret = gpiod_request(desc, con_id ?: devname);
 	if (ret) {
 		if (ret == -EBUSY && flags & GPIOD_FLAGS_BIT_NONEXCLUSIVE) {
 			/*
 			 * This happens when there are several consumers for
 			 * the same GPIO line: we just return here without
-			 * further initialization. It is a bit if a hack.
+			 * further initialization. It is a bit of a hack.
 			 * This is necessary to support fixed regulators.
 			 *
 			 * FIXME: Make this more sane and safe.
 			 */
-			dev_info(dev, "nonexclusive access to GPIO for %s\n",
-				 con_id ? con_id : devname);
+			dev_info(dev, "nonexclusive access to GPIO for %s\n", con_id ?: devname);
 			return desc;
 		} else {
 			return ERR_PTR(ret);
-- 
2.34.1

[PATCH v3 4/4] gpiolib: Simplify error path in gpiod_get_index() when requesting GPIO

[Andy Shevchenko]

Simplify error path in the gpiod_get_index() when requesting a GPIO line by:
 - checking for error condition first
 - dropping redundant 'else'

As a result, decrease the indentation level for better readability.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index daedf8207173..b4d8bf3a1121 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3933,20 +3933,19 @@ struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
 	 */
 	ret = gpiod_request(desc, con_id ?: devname);
 	if (ret) {
-		if (ret == -EBUSY && flags & GPIOD_FLAGS_BIT_NONEXCLUSIVE) {
-			/*
-			 * This happens when there are several consumers for
-			 * the same GPIO line: we just return here without
-			 * further initialization. It is a bit of a hack.
-			 * This is necessary to support fixed regulators.
-			 *
-			 * FIXME: Make this more sane and safe.
-			 */
-			dev_info(dev, "nonexclusive access to GPIO for %s\n", con_id ?: devname);
-			return desc;
-		} else {
+		if (!(ret == -EBUSY && flags & GPIOD_FLAGS_BIT_NONEXCLUSIVE))
 			return ERR_PTR(ret);
-		}
+
+		/*
+		 * This happens when there are several consumers for
+		 * the same GPIO line: we just return here without
+		 * further initialization. It is a bit of a hack.
+		 * This is necessary to support fixed regulators.
+		 *
+		 * FIXME: Make this more sane and safe.
+		 */
+		dev_info(dev, "nonexclusive access to GPIO for %s\n", con_id ?: devname);
+		return desc;
 	}
 
 	ret = gpiod_configure_flags(desc, con_id, lookupflags, flags);
-- 
2.34.1

Re: [PATCH v3 0/4] gpiolib: A fix and a few cleanups

[Andy Shevchenko]

On Tue, Feb 01, 2022 at 05:27:54PM +0200, Andy Shevchenko wrote:
> Patch 1 is a fix for wrong error code to user space.
> Patches 2-4 are small cleanups.

> Can be routed via my tree, or directly into GPIO, whatever maintainers
> prefer.

Bart, if the series is okay for you tell me which route to take?

> Changelog v3:
> - joined two patches into a single fix (Bart)
> - added Rb tag (Linus)
> - renamed for_each_gpio_desc_with_flag() macro to be descriptive (Johan)
> - added two new cleanups
> 
> Andy Shevchenko (4):
>   gpiolib: Never return internal error codes to user space
>   gpiolib: Introduce for_each_gpio_desc_with_flag() macro
>   gpiolib: Use short form of ternary operator in gpiod_get_index()
>   gpiolib: Simplify error path in gpiod_get_index() when requesting GPIO
> 
>  drivers/gpio/gpiolib-cdev.c  |  6 +++---
>  drivers/gpio/gpiolib-of.c    | 10 ++++------
>  drivers/gpio/gpiolib-sysfs.c | 14 ++++----------
>  drivers/gpio/gpiolib.c       | 35 ++++++++++++++++-------------------
>  drivers/gpio/gpiolib.h       | 19 +++++++++++++++++++
>  5 files changed, 46 insertions(+), 38 deletions(-)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/4] gpiolib: A fix and a few cleanups

[Bartosz Golaszewski]

On Mon, Feb 7, 2022 at 3:44 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Feb 01, 2022 at 05:27:54PM +0200, Andy Shevchenko wrote:
> > Patch 1 is a fix for wrong error code to user space.
> > Patches 2-4 are small cleanups.
>
> > Can be routed via my tree, or directly into GPIO, whatever maintainers
> > prefer.
>
> Bart, if the series is okay for you tell me which route to take?
>

Queued the first one for fixes and 2-4 for v5.18.

Bart

Re: [PATCH v3 0/4] gpiolib: A fix and a few cleanups

[Andy Shevchenko]

On Tue, Feb 8, 2022 at 3:16 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> On Mon, Feb 7, 2022 at 3:44 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, Feb 01, 2022 at 05:27:54PM +0200, Andy Shevchenko wrote:
> > > Patch 1 is a fix for wrong error code to user space.
> > > Patches 2-4 are small cleanups.
> >
> > > Can be routed via my tree, or directly into GPIO, whatever maintainers
> > > prefer.
> >
> > Bart, if the series is okay for you tell me which route to take?
>
> Queued the first one for fixes and 2-4 for v5.18.

Thank you!

-- 
With Best Regards,
Andy Shevchenko