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: <20181128104350.31902-1-linus.walleij@linaro.org>
In-Reply-To: <20181128104350.31902-1-linus.walleij@linaro.org>
From:   Bartosz Golaszewski <bgolaszewski@baylibre.com>
Date:   Thu, 29 Nov 2018 19:38:20 +0100
Message-ID: <CAMpxmJUsAccSKELNxn6DEWO6_3aCbzM=hZyaXUCr30S0-Eh0mA@mail.gmail.com>
Subject: Re: [PATCH 00/10] Regulator ena_gpiod fixups
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
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

=C5=9Br., 28 lis 2018 o 11:43 Linus Walleij <linus.walleij@linaro.org> napi=
sa=C5=82(a):
>
> We noticed a refcounting fight between the kernel device
> core devm_* and the regulator core refcounting, pertaining
> to enable GPIOd:s that may be shared between multiple
> regulators.
>
> Fix this with a series that hand it all over to the
> regulator core and remove any devm_* stuff pertaining
> to these GPIOs.
>
> This includes a patch to gpiolib to make gpiod_get_from_node()
> available. Just go ahead and apply this to the regulator
> tree.
>
> If these need to go for fixes or not is up to the
> regulator maintainer, but all commits have a proper
> Fixes: tag in case they would. I have noted in the
> four last commits that the gpiolib patch is a
> prerequisite.
>
> Linus Walleij (10):
>   regulator: fixed: Let core handle GPIO descriptor
>   regulator: lm363x: Let core handle GPIO descriptor
>   regulator: lp8788-ldo: Let core handle GPIO descriptor
>   regulator: max8952: Let core handle GPIO descriptor
>   regulator: max8973: Let core handle GPIO descriptor
>   gpio: Export gpiod_get_from_of_node()
>   regulator: da9211: Let core handle GPIO descriptors
>   regulator: max77686: Let core handle GPIO descriptor
>   regulator: s5m8767: Let core handle GPIO descriptors
>   regulator: tps65090: Let core handle GPIO descriptors
>
>  drivers/gpio/gpiolib.h                 |  6 -----
>  drivers/regulator/da9211-regulator.c   |  4 +--
>  drivers/regulator/fixed.c              |  4 ++-
>  drivers/regulator/lm363x-regulator.c   |  8 ++++--
>  drivers/regulator/lp8788-ldo.c         |  4 ++-
>  drivers/regulator/max77686-regulator.c |  3 +--
>  drivers/regulator/max8952.c            |  8 +++---
>  drivers/regulator/max8973-regulator.c  | 12 ++++++---
>  drivers/regulator/s5m8767.c            | 37 ++++++++++++++++++--------
>  drivers/regulator/tps65090-regulator.c | 10 +++----
>  include/linux/gpio/consumer.h          | 13 +++++++++
>  11 files changed, 72 insertions(+), 37 deletions(-)
>
> --
> 2.19.1
>

I'm wondering if instead of using the non-devm variants of
gpiod_get_*() routines, we shouldn't provide helpers in the regulator
framework that would be named accordingly, for example:
regulator_gpiod_get_optional() etc. even if all they do is call the
relevant gpiolib function. Those helpers could then be documented as
passing the control over GPIO lines over to the regulator subsystem.

The reason for that is that most driver developers will automatically
use devm functions whenever available and having a single non-devm
function without any comment used in a driver normally using devres
looks like a bug. Expect people sending "fixes" in a couple months.

Bart