linux-hwmon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matti Vaittinen <mazziesaccount@gmail.com>
To: matti.vaittinen@fi.rohmeurope.com, mazziesaccount@gmail.com
Cc: Jonathan Corbet <corbet@lwn.net>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Neil Armstrong <narmstrong@baylibre.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Kevin Hilman <khilman@baylibre.com>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Jean Delvare <jdelvare@suse.com>,
	Guenter Roeck <linux@roeck-us.net>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Michael Hennerich <Michael.Hennerich@analog.com>,
	Alexandru Tachici <alexandru.tachici@analog.com>,
	Jonathan Cameron <jic23@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Matti Vaittinen <mazziesaccount@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Alexandru Ardelean <aardelean@deviqon.com>,
	Peter Rosin <peda@axentia.se>,
	Aswath Govindraju <a-govindraju@ti.com>,
	Johan Hovold <johan+linaro@kernel.org>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-clk@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-amlogic@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-hwmon@vger.kernel.org, linux-iio@vger.kernel.org
Subject: [RFC PATCH 0/7] Devm helpers for regulator get and enable
Date: Wed, 10 Aug 2022 14:21:18 +0300	[thread overview]
Message-ID: <cover.1660127865.git.mazziesaccount@gmail.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 4833 bytes --]

Devm helpers for regulator get and enable

First patch in the series is actually just a simple documentation fix
which could be taken in as it is now (even though the rest of the series
is a RFC).

A few* drivers seem to pattern demonstrated by pseudocode:

- devm_regulator_get()
- regulator_enable()
- devm_add_action_or_reset(regulator_disable())

(*) A rough idea what 'a few' means in this context can be get by issuing:
"git grep -In -A10 devm_regulator_get |grep -B5 -A5 add_action |less"
and then further checking some of the reported drivers. This is what I did
when I realized I needed to enable a regulator for accelerometer and
thought I'd go with devm-action...

Introducing devm helpers for this pattern would remove bunch of code from
drivers. Typically at least following:

- replace 3 calls (devm_regulator_get[_optional](), regulator_enable(),
  devm_add_action_or_reset()) with just one
  (devm_regulator_get_enable[_optional]()).
- drop disable callback.

I believe this simplifies things by removing some dublicated code.

The other RFC aspect besides the question if this actually is useful, is
whether the devm_regulator_get_enable[_optional]() should return a pointer
to the obtained regulator or not. This RFC version does not return the
pointer for user because any call to regulator_disable() may lead to
regulator enable count imbalance upon device detach. (Eg, if someone calls
regulator_disable() and the device is then detached before user has
re-enabled the regulator). Not returning the pointer to obtained regulator
to caller is a good hint that the enable/disable should not be manually
handled when this API is used.

OTOH, not returning the pointer reduces the use-cases by not allowing
the consumers to perform other regulator actions. For example request the
voltages. A few drivers which used the "get, enable,
devm_action_to_disable" did also query the voltages. The suggested form of
the API does not suit needs of such users. The new API in its current form
really allows to only cover the very dummy cases where regulator is only
enabled for a lifetime of the driver. I am unsure if this is really
beneficial (well, there seems to be bunch of drivers doing just this) - or
if we should go with a version returning the struct regulator *

Some drivers did also manually disable the regulator (even though they had
registered the devm-action for disable) for PM functionality. I am unsure
if such use for suspend is actually safe(?) I didn't check if we can
guarantee that the driver is not detached after the PM suspend has disabled
the regulator(?)

This RFC converts only few a drivers to demonstrate benefits. This makes it
easier to rework the series if people thinks returning the pointer to
struct regulator should be done. I can't promise I'll convert all drivers
so, there is still plenty of fish in the sea for people who like to improve
the code (or count the beans ;]).

Finally - most of the converted drivers have not been tested (other than
compile-tested) due to lack of HW. All reviews and testing is _highly_
appreciated (as always!). I have the driver changes in individual patches
to make reviewing easier. I will squash the driver changes into one patch /
subsystem when I'll drop the "RFC" from the series.

Patch 1:
	Fix docmentation (devres API list) for regulator APIs
Patch 2:
	The devm helpers.
Patch 3:
	Add new devm-helper APIs to docs.
Patches 4 ... 7:
	Example drivers.

---

Matti Vaittinen (7):
  docs: devres: regulator: Add missing devm_* functions to devres.rst
  regulator: Add devm helpers for get and enable
  docs: devres: regulator: Add new get_enable functions to devres.rst
  clk: cdce925: simplify using devm_regulator_get_enable()
  gpu: drm: meson: simplify using devm_regulator_get_enable_optional()
  hwmon: lm90: simplify using devm_regulator_get_enable()
  adc: ad7192: simplify using devm_regulator_get_enable()

 .../driver-api/driver-model/devres.rst        |  9 +++
 drivers/clk/clk-cdce925.c                     | 21 ++-----
 drivers/gpu/drm/meson/meson_dw_hdmi.c         | 23 +-------
 drivers/hwmon/lm90.c                          | 21 +------
 drivers/iio/adc/ad7192.c                      | 15 +----
 drivers/regulator/devres.c                    | 59 +++++++++++++++++++
 include/linux/regulator/consumer.h            | 13 ++++
 7 files changed, 92 insertions(+), 69 deletions(-)

-- 
2.37.1


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

             reply	other threads:[~2022-08-10 11:21 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-10 11:21 Matti Vaittinen [this message]
2022-08-10 11:32 ` [RFC PATCH 6/7] hwmon: lm90: simplify using devm_regulator_get_enable() Matti Vaittinen
2022-08-10 12:52   ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1660127865.git.mazziesaccount@gmail.com \
    --to=mazziesaccount@gmail.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=a-govindraju@ti.com \
    --cc=aardelean@deviqon.com \
    --cc=airlied@linux.ie \
    --cc=alexandru.tachici@analog.com \
    --cc=broonie@kernel.org \
    --cc=corbet@lwn.net \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jbrunet@baylibre.com \
    --cc=jdelvare@suse.com \
    --cc=jic23@kernel.org \
    --cc=johan+linaro@kernel.org \
    --cc=khilman@baylibre.com \
    --cc=lars@metafoo.de \
    --cc=lgirdwood@gmail.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=matti.vaittinen@fi.rohmeurope.com \
    --cc=mturquette@baylibre.com \
    --cc=narmstrong@baylibre.com \
    --cc=peda@axentia.se \
    --cc=sboyd@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).