On Fri, Jan 03, 2014 at 08:07:22PM +0900, Alexandre Courbot wrote:

On Thu, Jan 2, 2014 at 10:07 PM, Linus Walleij [off-list ref] wrote:

quoted

On Sat, Dec 21, 2013 at 11:49 AM, Rashika Kheria
[off-list ref] wrote:

quoted

-/**
- * devm_gpiod_put - Resource-managed gpiod_put()
- * @desc:      GPIO descriptor to dispose of
- *
- * Dispose of a GPIO descriptor obtained with devm_gpiod_get() or
- * devm_gpiod_get_index(). Normally this function will not be called as the GPIO
- * will be disposed of by the resource management code.
- */
-void devm_gpiod_put(struct device *dev, struct gpio_desc *desc)
-{
-       WARN_ON(devres_release(dev, devm_gpiod_release, devm_gpiod_match,
-               &desc));
-}
-EXPORT_SYMBOL(devm_gpiod_put);

Alexandre what do you think about this? Can we think of a scenario
where explicit garbage collection is going to be needed or should we
remove this for now?

Sorry for the delayed reply, I quickly saw the patch and then the
holidays got in the way. :)

I think all these functions should be kept. It is true that they are
seldomly used, and that the purpose of devm is to garbage-collect
resources upon driver removal, but they might (actually, probably
will) become needed by someone at some point in the future. One
example I can think of is two drivers that collaborate to share the
same GPIO line. If they acquire the GPIO through devm_gpiod_get() they
will need devm_gpiod_put() to release it so the other driver can
acquire it.

On a more general note, devm_clk_put(), devm_regulator_put(),
devm_pinctrl_put() and probably others devm_*_put() functions are
actively used in the kernel, to  support the idea that a devm removal
function makes sense. That not all the subsystem-provided functions
are used by mainline drivers does not necessary mean they should be
removed, especially if they serve a purpose. We should keep our APIs
consistent and future-proof, not to mention out-of-tree drivers that
may use them.

So this patch is a nack as far as I'm concerned, not only the GPIO
part, but the whole of it.

As far as I can tell, the few calls to the devm_*_put functions I can
see look like unnecessary calls as part of driver/device
uninitialization, and could be removed either directly or with minor
reworking.  That there are only 1-2 calls to each in the entire kernel
is also quite telling.

In any case, it's disappointing to have a pile of unused functions in
the kernel on the theory that they *might* be needed; it's not like it'd
be hard to retrieve them from git if they're ever needed.

However, if you're insistent on keeping them, it'd be easy enough to
provide patches to include an appropriate header with prototypes for
them instead, which would also eliminate the warning this patch series
set out to eliminate.

- Josh Triplett