Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20181201154151.14890-1-linus.walleij@linaro.org> <20181201154151.14890-10-linus.walleij@linaro.org>
In-Reply-To: <20181201154151.14890-10-linus.walleij@linaro.org>
From:   Bartosz Golaszewski <bgolaszewski@baylibre.com>
Date:   Sun, 2 Dec 2018 18:02:42 +0100
Message-ID: <CAMpxmJXoZOJkKJRL0pO-LJ4HXxWb4qj7iADbpEoS=kfL4h-jHQ@mail.gmail.com>
Subject: Re: [PATCH 09/13 v2] gpio: Add devm_gpiod_unhinge()
To:     Linus Walleij <linus.walleij@linaro.org>
Cc:     Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        ckeepax@opensource.cirrus.com, m.szyprowski@samsung.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

sob., 1 gru 2018 o 16:53 Linus Walleij <linus.walleij@linaro.org> napisa=C5=
=82(a):
>
> This adds a function named devm_gpiod_unhinge() that removes
> the resource management from a GPIO descriptor.
>
> I am not sure if this is the best anglosaxon name for the
> function, no other managed resources have an equivalent
> currently, but I chose "unhinge" as the closest intuitive
> thing I could imagine that fits Rusty Russell's API design
> criterions "the obvious use is the correct one" and
> "the name tells you how to use it".
>
> The idea came out of a remark from Mark Brown that it should
> be possible to handle over management of a resource from
> devres to the regulator core, and indeed we can do that.
>
> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - New patch to be able to hand over GPIO descriptors to
>   the regulator core.
> - Mark: feel free to apply this to the regulator tree with
>   the regulator tree with the rest.
> ---
>  Documentation/driver-model/devres.txt |  1 +
>  drivers/gpio/gpiolib-devres.c         | 17 +++++++++++++++++
>  include/linux/gpio/consumer.h         | 10 ++++++++++
>  3 files changed, 28 insertions(+)
>
> diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver=
-model/devres.txt
> index 43681ca0837f..fc4cc24dfb97 100644
> --- a/Documentation/driver-model/devres.txt
> +++ b/Documentation/driver-model/devres.txt
> @@ -254,6 +254,7 @@ GPIO
>    devm_gpiod_get_index_optional()
>    devm_gpiod_get_optional()
>    devm_gpiod_put()
> +  devm_gpiod_unhinge()
>    devm_gpiochip_add_data()
>    devm_gpiochip_remove()
>    devm_gpio_request()
> diff --git a/drivers/gpio/gpiolib-devres.c b/drivers/gpio/gpiolib-devres.=
c
> index 01959369360b..5864e758d7f2 100644
> --- a/drivers/gpio/gpiolib-devres.c
> +++ b/drivers/gpio/gpiolib-devres.c
> @@ -320,6 +320,23 @@ void devm_gpiod_put(struct device *dev, struct gpio_=
desc *desc)
>  }
>  EXPORT_SYMBOL(devm_gpiod_put);
>
> +/**
> + * devm_gpiod_unhinge - Remove resource management from a gpio descripto=
r
> + * @dev:       GPIO consumer
> + * @desc:      GPIO descriptor to remove resource management from
> + *
> + * Remove resource management from a GPIO descriptor. This is needed whe=
n
> + * you want to hand over lifecycle management of a descriptor to another
> + * mechanism.
> + */
> +
> +void devm_gpiod_unhinge(struct device *dev, struct gpio_desc *desc)
> +{
> +       WARN_ON(devres_destroy(dev, devm_gpiod_release,
> +                              devm_gpiod_match, desc));
> +}
> +EXPORT_SYMBOL(devm_gpiod_unhinge);
> +
>  /**
>   * devm_gpiod_put_array - Resource-managed gpiod_put_array()
>   * @dev:       GPIO consumer
> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.=
h
> index 348885f2f3d3..8aebcf822082 100644
> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -104,6 +104,7 @@ struct gpio_descs *__must_check
>  devm_gpiod_get_array_optional(struct device *dev, const char *con_id,
>                               enum gpiod_flags flags);
>  void devm_gpiod_put(struct device *dev, struct gpio_desc *desc);
> +void devm_gpiod_unhinge(struct device *dev, struct gpio_desc *desc);
>  void devm_gpiod_put_array(struct device *dev, struct gpio_descs *descs);
>
>  int gpiod_get_direction(struct gpio_desc *desc);
> @@ -249,6 +250,15 @@ static inline void gpiod_put(struct gpio_desc *desc)
>         WARN_ON(1);
>  }
>
> +static inline void devm_gpiod_unhinge(struct device *dev,
> +                                     struct gpio_desc *desc)
> +{
> +       might_sleep();
> +
> +       /* GPIO can never have been requested */
> +       WARN_ON(1);
> +}
> +
>  static inline void gpiod_put_array(struct gpio_descs *descs)
>  {
>         might_sleep();
> --
> 2.19.1
>

Yes! This is much better than simply using non-devm variants of gpiod_get()=
.

Reviewed-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>