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

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Wed, Jan 25, 2017 at 1:06 AM, Furquan Shaikh <furquan@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> Signed-off-by: Furquan Shaikh <furquan@xxxxxxxxxxxx>

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-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html