Re: [PATCH] drivers: Remove unused devm_*_put functions

[Josh Triplett <josh@joshtriplett.org>]

On Fri, Jan 03, 2014 at 08:07:22PM +0900, Alexandre Courbot wrote:
> On Thu, Jan 2, 2014 at 10:07 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Sat, Dec 21, 2013 at 11:49 AM, Rashika Kheria
> > <rashika.kheria@gmail.com> wrote:
> >> -/**
> >> - * 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