diff mbox

[6/7] drivers/gpio: Add and export gpiod_lookup[_index]

Message ID 20170125000641.25520-7-furquan@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Furquan Shaikh Jan. 25, 2017, 12:06 a.m. UTC 
Sometimes (as the case with fixed regulator) we only want to look up,
but not necessarily reserve right away, GPIO. gpiod_lookup() and
gpiod_lookup_by_index() allow us doing just that.

Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Furquan Shaikh <furquan@chromium.org>
---
 drivers/gpio/gpiolib.c        | 75 ++++++++++++++++++++++++++++++++++---------
 include/linux/gpio/consumer.h | 21 ++++++++++++
 2 files changed, 80 insertions(+), 16 deletions(-)

Comments

kernel test robot Jan. 25, 2017, 11:18 a.m. UTC | #1 
Hi Furquan,

[auto build test ERROR on regulator/for-next]
[also build test ERROR on v4.10-rc5 next-20170125]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Furquan-Shaikh/Implement-generic-regulator-constraints-parsing-for-ACPI-and-OF/20170125-171807
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
config: i386-randconfig-x004-201704 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpio/gpiolib.c: In function 'gpiod_lookup_index':
>> drivers/gpio/gpiolib.c:3221:44: error: 'flags' undeclared (first use in this function)
       desc = acpi_find_gpio(dev, con_id, idx, flags, &lookupflags);
                                               ^~~~~
   drivers/gpio/gpiolib.c:3221:44: note: each undeclared identifier is reported only once for each function it appears in

vim +/flags +3221 drivers/gpio/gpiolib.c

bae48da23 Alexandre Courbot 2013-10-17  3215  		/* Using device tree? */
4d8440b93 Rafael J. Wysocki 2015-03-10  3216  		if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
bae48da23 Alexandre Courbot 2013-10-17  3217  			dev_dbg(dev, "using device tree for GPIO lookup\n");
39b2bbe3d Alexandre Courbot 2014-07-25  3218  			desc = of_find_gpio(dev, con_id, idx, &lookupflags);
4d8440b93 Rafael J. Wysocki 2015-03-10  3219  		} else if (ACPI_COMPANION(dev)) {
81f59e9d1 Mika Westerberg   2013-10-10  3220  			dev_dbg(dev, "using ACPI for GPIO lookup\n");
25487533f Dmitry Torokhov   2016-03-24 @3221  			desc = acpi_find_gpio(dev, con_id, idx, flags, &lookupflags);
35c5d7fdc Alexandre Courbot 2013-11-23  3222  		}
4d8440b93 Rafael J. Wysocki 2015-03-10  3223  	}
35c5d7fdc Alexandre Courbot 2013-11-23  3224  

:::::: The code at line 3221 was first introduced by commit
:::::: 25487533ff311014b0cc39ed7fd721c114e1b11f gpio / ACPI: ignore GpioInt() GPIOs when requesting GPIO_OUT_*

:::::: TO: Dmitry Torokhov <dtor@chromium.org>
:::::: CC: Linus Walleij <linus.walleij@linaro.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Linus Walleij Jan. 26, 2017, 3:24 p.m. UTC | #2 
On Wed, Jan 25, 2017 at 1:06 AM, Furquan Shaikh <furquan@chromium.org> wrote:

> Sometimes (as the case with fixed regulator) we only want to look up,
> but not necessarily reserve right away, GPIO. gpiod_lookup() and
> gpiod_lookup_by_index() allow us doing just that.
>
> Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
> Signed-off-by: Furquan Shaikh <furquan@chromium.org>

Overall if this solves the GPIO regulator mess, I'd be happy
to apply it.

But overall it is a bit scary with looking up something struct gpiod *
that does not at all have the same semantics as a gpiod *
you get from gpiod_get().

We need to make sure that a gpiod looked up this way does not
work and tick and cannot perform operations as one obtained
with a proper gpiod_get().

>  /**
> + * gpiod_lookup - look up a GPIO for a given GPIO function
> + * @dev:       GPIO consumer, can be NULL for system-global GPIOs
> + * @con_id:    function within the GPIO consumer
> + *
> + * Return the GPIO descriptor corresponding to the function con_id of device
> + * dev, -ENOENT if no GPIO has been assigned to the requested function, or
> + * another IS_ERR() code.
> + */
> +struct gpio_desc *__must_check gpiod_lookup(struct device *dev,
> +                                           const char *con_id)
> +{
> +       return gpiod_lookup_index(dev, con_id, 0);
> +}
> +EXPORT_SYMBOL_GPL(gpiod_lookup);

Make this a static inline in the <linux/gpio/consumer.h> header instead.
Saves a lot of trouble.

>  /**
> - * gpiod_get_index - obtain a GPIO from a multi-index GPIO function
> + * gpiod_lookup_index - look up a GPIO from a multi-index GPIO function
>   * @dev:       GPIO consumer, can be NULL for system-global GPIOs
>   * @con_id:    function within the GPIO consumer
>   * @idx:       index of the GPIO to obtain in the consumer
> - * @flags:     optional GPIO initialization flags
> - *
> - * This variant of gpiod_get() allows to access GPIOs other than the first
> - * defined one for functions that define several GPIOs.
>   *
> - * Return a valid GPIO descriptor, -ENOENT if no GPIO has been assigned to the
> - * requested function and/or index, or another IS_ERR() code if an error
> - * occurred while trying to acquire the GPIO.
> + * Return a valid GPIO descriptor, or -ENOENT if no GPIO has been assigned to
> + * the requested function and/or index, or another IS_ERR() code.
>   */

What a mess in the patch. Not your fault but format-patch created this
mess.0

> -struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
> -                                              const char *con_id,
> -                                              unsigned int idx,
> -                                              enum gpiod_flags flags)
> +struct gpio_desc *__must_check gpiod_lookup_index(struct device *dev,
> +                                                 const char *con_id,
> +                                                 unsigned int idx)
>  {
>         struct gpio_desc *desc = NULL;
> -       int status;
>         enum gpio_lookup_flags lookupflags = 0;
>
>         dev_dbg(dev, "GPIO lookup for consumer %s\n", con_id);
> @@ -3285,16 +3294,50 @@ struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
>                 desc = gpiod_find(dev, con_id, idx, &lookupflags);
>         }
>
> -       if (IS_ERR(desc)) {
> +       if (IS_ERR(desc))
>                 dev_dbg(dev, "lookup for GPIO %s failed\n", con_id);
> +
> +       /*
> +        * Configure static flags based on lookup data (such as
> +        * "active low", "open drain", etc.)
> +        */
> +       gpiod_configure_flags(desc, con_id, lookupflags, 0);

So what about setting some special flag from
gpiolib.h like
set_bit(FLAG_ONLY_LOOKUP, &desc->flags);
here numbing all
operations on the returned gpiod, by a check in
VALIDATE_DESC() and VALIDATE_DESC_VOID()
so that no real operations fall through?

Then that flag will just be forcibly cleared by
gpiod_get[_index]() later.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a07ae9e37930..7f8ac6c229e8 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3165,6 +3165,22 @@  int gpiod_count(struct device *dev, const char *con_id)
 EXPORT_SYMBOL_GPL(gpiod_count);
 
 /**
+ * gpiod_lookup - look up a GPIO for a given GPIO function
+ * @dev:	GPIO consumer, can be NULL for system-global GPIOs
+ * @con_id:	function within the GPIO consumer
+ *
+ * Return the GPIO descriptor corresponding to the function con_id of device
+ * dev, -ENOENT if no GPIO has been assigned to the requested function, or
+ * another IS_ERR() code.
+ */
+struct gpio_desc *__must_check gpiod_lookup(struct device *dev,
+					    const char *con_id)
+{
+	return gpiod_lookup_index(dev, con_id, 0);
+}
+EXPORT_SYMBOL_GPL(gpiod_lookup);
+
+/**
  * gpiod_get - obtain a GPIO for a given GPIO function
  * @dev:	GPIO consumer, can be NULL for system-global GPIOs
  * @con_id:	function within the GPIO consumer
@@ -3241,26 +3257,19 @@  static int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
 }
 
 /**
- * gpiod_get_index - obtain a GPIO from a multi-index GPIO function
+ * gpiod_lookup_index - look up a GPIO from a multi-index GPIO function
  * @dev:	GPIO consumer, can be NULL for system-global GPIOs
  * @con_id:	function within the GPIO consumer
  * @idx:	index of the GPIO to obtain in the consumer
- * @flags:	optional GPIO initialization flags
- *
- * This variant of gpiod_get() allows to access GPIOs other than the first
- * defined one for functions that define several GPIOs.
  *
- * Return a valid GPIO descriptor, -ENOENT if no GPIO has been assigned to the
- * requested function and/or index, or another IS_ERR() code if an error
- * occurred while trying to acquire the GPIO.
+ * Return a valid GPIO descriptor, or -ENOENT if no GPIO has been assigned to
+ * the requested function and/or index, or another IS_ERR() code.
  */
-struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
-					       const char *con_id,
-					       unsigned int idx,
-					       enum gpiod_flags flags)
+struct gpio_desc *__must_check gpiod_lookup_index(struct device *dev,
+						  const char *con_id,
+						  unsigned int idx)
 {
 	struct gpio_desc *desc = NULL;
-	int status;
 	enum gpio_lookup_flags lookupflags = 0;
 
 	dev_dbg(dev, "GPIO lookup for consumer %s\n", con_id);
@@ -3285,16 +3294,50 @@  struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
 		desc = gpiod_find(dev, con_id, idx, &lookupflags);
 	}
 
-	if (IS_ERR(desc)) {
+	if (IS_ERR(desc))
 		dev_dbg(dev, "lookup for GPIO %s failed\n", con_id);
+
+	/*
+	 * Configure static flags based on lookup data (such as
+	 * "active low", "open drain", etc.)
+	 */
+	gpiod_configure_flags(desc, con_id, lookupflags, 0);
+
+	return desc;
+}
+EXPORT_SYMBOL_GPL(gpiod_lookup_index);
+
+/**
+ * gpiod_get_index - obtain a GPIO from a multi-index GPIO function
+ * @dev:	GPIO consumer, can be NULL for system-global GPIOs
+ * @con_id:	function within the GPIO consumer
+ * @idx:	index of the GPIO to obtain in the consumer
+ * @flags:	optional GPIO initialization flags
+ *
+ * This variant of gpiod_get() allows to access GPIOs other than the first
+ * defined one for functions that define several GPIOs.
+ *
+ * Return a valid GPIO descriptor, -ENOENT if no GPIO has been assigned to the
+ * requested function and/or index, or another IS_ERR() code if an error
+ * occurred while trying to acquire the GPIO.
+ */
+struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
+					       const char *con_id,
+					       unsigned int idx,
+					       enum gpiod_flags flags)
+{
+	struct gpio_desc *desc;
+	int status;
+
+	desc = gpiod_lookup_index(dev, con_id, idx);
+	if (IS_ERR(desc))
 		return desc;
-	}
 
 	status = gpiod_request(desc, con_id);
 	if (status < 0)
 		return ERR_PTR(status);
 
-	status = gpiod_configure_flags(desc, con_id, lookupflags, flags);
+	status = gpiod_configure_flags(desc, con_id, 0, flags);
 	if (status < 0) {
 		dev_dbg(dev, "setup of GPIO %s failed\n", con_id);
 		gpiod_put(desc);
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index fb0fde686cb1..8fcb638f5568 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -46,6 +46,13 @@  enum gpiod_flags {
 /* Return the number of GPIOs associated with a device / function */
 int gpiod_count(struct device *dev, const char *con_id);
 
+/* Look up GPIOs */
+struct gpio_desc *__must_check gpiod_lookup(struct device *dev,
+					    const char *con_id);
+struct gpio_desc *__must_check gpiod_lookup_index(struct device *dev,
+						  const char *con_id,
+						  unsigned int idx);
+
 /* Acquire and dispose GPIOs */
 struct gpio_desc *__must_check gpiod_get(struct device *dev,
 					 const char *con_id,
@@ -146,6 +153,20 @@  static inline int gpiod_count(struct device *dev, const char *con_id)
 	return 0;
 }
 
+static inline struct gpio_desc *__must_check gpiod_lookup(struct device *dev,
+							  const char *con_id)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline struct gpio_desc *__must_check
+gpiod_lookup_index(struct device *dev,
+		   const char *con_id,
+		   unsigned int idx)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
 static inline struct gpio_desc *__must_check gpiod_get(struct device *dev,
 						       const char *con_id,
 						       enum gpiod_flags flags)