linux-kernel - Re: [PATCH 00/10] Regulator ena

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMpxmJUsAccSKELNxn6DEWO6_3aCbzM=hZyaXUCr30S0-Eh0mA@mail.gmail.com>
Date:   Thu, 29 Nov 2018 19:38:20 +0100
From:   Bartosz Golaszewski <bgolaszewski@...libre.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        ckeepax@...nsource.cirrus.com
Subject: Re: [PATCH 00/10] Regulator ena_gpiod fixups

śr., 28 lis 2018 o 11:43 Linus Walleij <linus.walleij@...aro.org> napisał(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