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.
A few* drivers seem to use pattern demonstrated by pseudocode:
- devm_regulator_get()
- regulator_enable()
- devm_add_action_or_reset(regulator_disable())
Introducing devm helpers for this pattern would remove bunch of code from
drivers. Typically 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.
- remove stored pointer to struct regulator - which can lead to problem
when an devm action for regulator_disable is used.
I believe this simplifies things by removing some dublicated code.
The suggested managed 'get_enable' APIs do not return the pointer to
regulators for user because any call to regulator_disable()
(or regulator_enable()) may easily 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 these APIs are 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 API does not suit
needs of such users.
This series reowrks only a few drivers as I am short of time. 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!).
Revision history:
RFCv1 => v2:
- Add devm_regulator_bulk_get_enable() and
devm_regulator_bulk_put()
- Convert a couple of drivers to use the new
devm_regulator_bulk_get_enable().
- Squash all IIO patches into one.
Patch 1:
Fix docmentation (devres API list) for regulator APIs
Patch 2:
The new devm helpers.
Patch 3:
Add new devm-helper APIs to docs.
Patch 4:
simplified CLK driver(s)
Patch 5:
simplified GPU driver(s)
Patch 6:
simplified hwmon driver(s)
Patch 7:
simplified IIO driver(s)
---
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: simplify drivers using devm_regulator_*get_enable*()
hwmon: lm90: simplify using devm_regulator_get_enable()
iio: Simplify drivers using devm_regulator_*get_enable()
.../driver-api/driver-model/devres.rst | 11 ++
drivers/clk/clk-cdce925.c | 21 +--
drivers/gpu/drm/bridge/sii902x.c | 22 +--
drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +--
drivers/hwmon/lm90.c | 21 +--
drivers/iio/adc/ad7192.c | 15 +-
drivers/iio/dac/ltc2688.c | 23 +--
drivers/iio/gyro/bmg160_core.c | 24 +--
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 2 -
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 30 +---
drivers/regulator/devres.c | 164 ++++++++++++++++++
include/linux/regulator/consumer.h | 27 +++
12 files changed, 227 insertions(+), 156 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 =]
adc/ad7192:
Use devm_regulator_get_enable() instead of open coded get, enable,
add-action-to-disable-at-detach - pattern. Also drop the seemingly unused
struct member 'dvdd'.
dac/ltc2688:
gyro/bmg160_core:
Use devm_regulator_bulk_get_enable() instead of open coded bulk-get,
bulk-enable, add-action-to-disable-at-detach - pattern.
imu/st_lsm6dsx:
Use devm_regulator_bulk_get_enable() instead of open coded bulk-get,
bulk-enable, add-action-to-disable-at-detach - pattern.
A functional change (which seems like a bugfix) is that if
regulator_bulk_get fails, the enable is not attempted.
Signed-off-by: Matti Vaittinen <[email protected]>
---
RFCv1 => v2:
Squashed all IIO changes to one patch. Added use of
devm_regulator_bulk_get_enable().
Please note - this is only compile-tested due to the lack of HW. Careful
review and testing is _highly_ appreciated.
---
drivers/iio/adc/ad7192.c | 15 ++--------
drivers/iio/dac/ltc2688.c | 23 ++-------------
drivers/iio/gyro/bmg160_core.c | 24 ++--------------
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 2 --
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 30 ++++----------------
5 files changed, 13 insertions(+), 81 deletions(-)
diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index d71977be7d22..8a52c0dec3f9 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -177,7 +177,6 @@ struct ad7192_chip_info {
struct ad7192_state {
const struct ad7192_chip_info *chip_info;
struct regulator *avdd;
- struct regulator *dvdd;
struct clk *mclk;
u16 int_vref_mv;
u32 fclk;
@@ -1015,19 +1014,9 @@ static int ad7192_probe(struct spi_device *spi)
if (ret)
return ret;
- st->dvdd = devm_regulator_get(&spi->dev, "dvdd");
- if (IS_ERR(st->dvdd))
- return PTR_ERR(st->dvdd);
-
- ret = regulator_enable(st->dvdd);
- if (ret) {
- dev_err(&spi->dev, "Failed to enable specified DVdd supply\n");
- return ret;
- }
-
- ret = devm_add_action_or_reset(&spi->dev, ad7192_reg_disable, st->dvdd);
+ ret = devm_regulator_get_enable(&spi->dev, "dvdd");
if (ret)
- return ret;
+ return dev_err_probe(&spi->dev, ret, "Failed to enable specified DVdd supply\n");
ret = regulator_get_voltage(st->avdd);
if (ret < 0) {
diff --git a/drivers/iio/dac/ltc2688.c b/drivers/iio/dac/ltc2688.c
index 937b0d25a11c..1abc88cd99f9 100644
--- a/drivers/iio/dac/ltc2688.c
+++ b/drivers/iio/dac/ltc2688.c
@@ -84,7 +84,6 @@ struct ltc2688_chan {
struct ltc2688_state {
struct spi_device *spi;
struct regmap *regmap;
- struct regulator_bulk_data regulators[2];
struct ltc2688_chan channels[LTC2688_DAC_CHANNELS];
struct iio_chan_spec *iio_chan;
/* lock to protect against multiple access to the device and shared data */
@@ -902,13 +901,6 @@ static int ltc2688_setup(struct ltc2688_state *st, struct regulator *vref)
LTC2688_CONFIG_EXT_REF);
}
-static void ltc2688_disable_regulators(void *data)
-{
- struct ltc2688_state *st = data;
-
- regulator_bulk_disable(ARRAY_SIZE(st->regulators), st->regulators);
-}
-
static void ltc2688_disable_regulator(void *regulator)
{
regulator_disable(regulator);
@@ -970,6 +962,7 @@ static int ltc2688_probe(struct spi_device *spi)
struct regulator *vref_reg;
struct device *dev = &spi->dev;
int ret;
+ static const char * const regulators[] = {"vcc", "iovcc"};
indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
if (!indio_dev)
@@ -988,21 +981,11 @@ static int ltc2688_probe(struct spi_device *spi)
return dev_err_probe(dev, PTR_ERR(st->regmap),
"Failed to init regmap");
- st->regulators[0].supply = "vcc";
- st->regulators[1].supply = "iovcc";
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(st->regulators),
- st->regulators);
- if (ret)
- return dev_err_probe(dev, ret, "Failed to get regulators\n");
-
- ret = regulator_bulk_enable(ARRAY_SIZE(st->regulators), st->regulators);
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators),
+ regulators);
if (ret)
return dev_err_probe(dev, ret, "Failed to enable regulators\n");
- ret = devm_add_action_or_reset(dev, ltc2688_disable_regulators, st);
- if (ret)
- return ret;
-
vref_reg = devm_regulator_get_optional(dev, "vref");
if (IS_ERR(vref_reg)) {
if (PTR_ERR(vref_reg) != -ENODEV)
diff --git a/drivers/iio/gyro/bmg160_core.c b/drivers/iio/gyro/bmg160_core.c
index 81a6d09788bd..acfabac645b6 100644
--- a/drivers/iio/gyro/bmg160_core.c
+++ b/drivers/iio/gyro/bmg160_core.c
@@ -93,7 +93,6 @@
struct bmg160_data {
struct regmap *regmap;
- struct regulator_bulk_data regulators[2];
struct iio_trigger *dready_trig;
struct iio_trigger *motion_trig;
struct iio_mount_matrix orientation;
@@ -1067,19 +1066,13 @@ static const char *bmg160_match_acpi_device(struct device *dev)
return dev_name(dev);
}
-static void bmg160_disable_regulators(void *d)
-{
- struct bmg160_data *data = d;
-
- regulator_bulk_disable(ARRAY_SIZE(data->regulators), data->regulators);
-}
-
int bmg160_core_probe(struct device *dev, struct regmap *regmap, int irq,
const char *name)
{
struct bmg160_data *data;
struct iio_dev *indio_dev;
int ret;
+ static const char * const regulators[] = {"vdd", "vddio"};
indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
if (!indio_dev)
@@ -1090,22 +1083,11 @@ int bmg160_core_probe(struct device *dev, struct regmap *regmap, int irq,
data->irq = irq;
data->regmap = regmap;
- data->regulators[0].supply = "vdd";
- data->regulators[1].supply = "vddio";
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(data->regulators),
- data->regulators);
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators),
+ regulators);
if (ret)
return dev_err_probe(dev, ret, "Failed to get regulators\n");
- ret = regulator_bulk_enable(ARRAY_SIZE(data->regulators),
- data->regulators);
- if (ret)
- return ret;
-
- ret = devm_add_action_or_reset(dev, bmg160_disable_regulators, data);
- if (ret)
- return ret;
-
ret = iio_read_mount_matrix(dev, &data->orientation);
if (ret)
return ret;
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index a86dd29a4738..03238c64c777 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -372,7 +372,6 @@ struct st_lsm6dsx_sensor {
* struct st_lsm6dsx_hw - ST IMU MEMS hw instance
* @dev: Pointer to instance of struct device (I2C or SPI).
* @regmap: Register map of the device.
- * @regulators: VDD/VDDIO voltage regulators.
* @irq: Device interrupt line (I2C or SPI).
* @fifo_lock: Mutex to prevent concurrent access to the hw FIFO.
* @conf_lock: Mutex to prevent concurrent FIFO configuration update.
@@ -395,7 +394,6 @@ struct st_lsm6dsx_sensor {
struct st_lsm6dsx_hw {
struct device *dev;
struct regmap *regmap;
- struct regulator_bulk_data regulators[2];
int irq;
struct mutex fifo_lock;
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 910397716833..7b40f6b58834 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -2172,36 +2172,20 @@ static int st_lsm6dsx_irq_setup(struct st_lsm6dsx_hw *hw)
static int st_lsm6dsx_init_regulators(struct device *dev)
{
- struct st_lsm6dsx_hw *hw = dev_get_drvdata(dev);
int err;
-
/* vdd-vddio power regulators */
- hw->regulators[0].supply = "vdd";
- hw->regulators[1].supply = "vddio";
- err = devm_regulator_bulk_get(dev, ARRAY_SIZE(hw->regulators),
- hw->regulators);
- if (err)
- return dev_err_probe(dev, err, "failed to get regulators\n");
+ static const char * const regulators[] = {"vdd", "vddio"};
- err = regulator_bulk_enable(ARRAY_SIZE(hw->regulators),
- hw->regulators);
- if (err) {
- dev_err(dev, "failed to enable regulators: %d\n", err);
- return err;
- }
+ err = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators),
+ regulators);
+ if (err)
+ return dev_err_probe(dev, err, "failed to enable regulators\n");
msleep(50);
return 0;
}
-static void st_lsm6dsx_chip_uninit(void *data)
-{
- struct st_lsm6dsx_hw *hw = data;
-
- regulator_bulk_disable(ARRAY_SIZE(hw->regulators), hw->regulators);
-}
-
int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
struct regmap *regmap)
{
@@ -2225,10 +2209,6 @@ int st_lsm6dsx_probe(struct device *dev, int irq, int hw_id,
if (err)
return err;
- err = devm_add_action_or_reset(dev, st_lsm6dsx_chip_uninit, hw);
- if (err)
- return err;
-
hw->buff = devm_kzalloc(dev, ST_LSM6DSX_BUFF_SIZE, GFP_KERNEL);
if (!hw->buff)
return -ENOMEM;
--
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 =]
Drop open-coded pattern: 'devm_regulator_get(), regulator_enable(),
add_action_or_reset(regulator_disable)' and use the
devm_regulator_get_enable().
Signed-off-by: Matti Vaittinen <[email protected]>
Acked-by: Guenter Roeck <[email protected]>
---
RFCv1 => v2:
- No changes
---
drivers/hwmon/lm90.c | 21 ++-------------------
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index 3820f0e61510..2ab561ec367c 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -1848,12 +1848,6 @@ static void lm90_remove_pec(void *dev)
device_remove_file(dev, &dev_attr_pec);
}
-static void lm90_regulator_disable(void *regulator)
-{
- regulator_disable(regulator);
-}
-
-
static const struct hwmon_ops lm90_ops = {
.is_visible = lm90_is_visible,
.read = lm90_read,
@@ -1865,24 +1859,13 @@ static int lm90_probe(struct i2c_client *client)
struct device *dev = &client->dev;
struct i2c_adapter *adapter = client->adapter;
struct hwmon_channel_info *info;
- struct regulator *regulator;
struct device *hwmon_dev;
struct lm90_data *data;
int err;
- regulator = devm_regulator_get(dev, "vcc");
- if (IS_ERR(regulator))
- return PTR_ERR(regulator);
-
- err = regulator_enable(regulator);
- if (err < 0) {
- dev_err(dev, "Failed to enable regulator: %d\n", err);
- return err;
- }
-
- err = devm_add_action_or_reset(dev, lm90_regulator_disable, regulator);
+ err = devm_regulator_get_enable(dev, "vcc");
if (err)
- return err;
+ return dev_err_probe(dev, err, "Failed to enable regulator\n");
data = devm_kzalloc(dev, sizeof(struct lm90_data), GFP_KERNEL);
if (!data)
--
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 =]
Simplify drivers using managed "regulator get and enable".
meson:
Use the devm_regulator_get_enable_optional(). Also drop the seemingly
unused struct member 'hdmi_supply'.
sii902x:
Simplify using devm_regulator_bulk_get_enable()
Signed-off-by: Matti Vaittinen <[email protected]>
---
RFCv1 => v2:
- Change also sii902x to use devm_regulator_bulk_get_enable()
Please note - this is only compile-tested due to the lack of HW. Careful
review and testing is _highly_ appreciated.
---
drivers/gpu/drm/bridge/sii902x.c | 22 +++-------------------
drivers/gpu/drm/meson/meson_dw_hdmi.c | 23 +++--------------------
2 files changed, 6 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 65549fbfdc87..4bf572b7ca77 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -170,7 +170,6 @@ struct sii902x {
struct drm_connector connector;
struct gpio_desc *reset_gpio;
struct i2c_mux_core *i2cmux;
- struct regulator_bulk_data supplies[2];
bool sink_is_hdmi;
/*
* Mutex protects audio and video functions from interfering
@@ -1070,6 +1069,7 @@ static int sii902x_probe(struct i2c_client *client,
struct device *dev = &client->dev;
struct device_node *endpoint;
struct sii902x *sii902x;
+ static const char * const supplies[] = {"iovcc", "cvcc12"};
int ret;
ret = i2c_check_functionality(client->adapter,
@@ -1120,27 +1120,13 @@ static int sii902x_probe(struct i2c_client *client,
mutex_init(&sii902x->mutex);
- sii902x->supplies[0].supply = "iovcc";
- sii902x->supplies[1].supply = "cvcc12";
- ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- if (ret < 0)
- return ret;
-
- ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(supplies), supplies);
if (ret < 0) {
dev_err_probe(dev, ret, "Failed to enable supplies");
return ret;
}
- ret = sii902x_init(sii902x);
- if (ret < 0) {
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
- }
-
- return ret;
+ return sii902x_init(sii902x);
}
static int sii902x_remove(struct i2c_client *client)
@@ -1150,8 +1136,6 @@ static int sii902x_remove(struct i2c_client *client)
i2c_mux_del_adapters(sii902x->i2cmux);
drm_bridge_remove(&sii902x->bridge);
- regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
- sii902x->supplies);
return 0;
}
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 5cd2b2ebbbd3..7642f740272b 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -140,7 +140,6 @@ struct meson_dw_hdmi {
struct reset_control *hdmitx_apb;
struct reset_control *hdmitx_ctrl;
struct reset_control *hdmitx_phy;
- struct regulator *hdmi_supply;
u32 irq_stat;
struct dw_hdmi *hdmi;
struct drm_bridge *bridge;
@@ -665,11 +664,6 @@ static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi)
}
-static void meson_disable_regulator(void *data)
-{
- regulator_disable(data);
-}
-
static void meson_disable_clk(void *data)
{
clk_disable_unprepare(data);
@@ -723,20 +717,9 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
meson_dw_hdmi->data = match;
dw_plat_data = &meson_dw_hdmi->dw_plat_data;
- meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev, "hdmi");
- if (IS_ERR(meson_dw_hdmi->hdmi_supply)) {
- if (PTR_ERR(meson_dw_hdmi->hdmi_supply) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
- meson_dw_hdmi->hdmi_supply = NULL;
- } else {
- ret = regulator_enable(meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- ret = devm_add_action_or_reset(dev, meson_disable_regulator,
- meson_dw_hdmi->hdmi_supply);
- if (ret)
- return ret;
- }
+ ret = devm_regulator_get_enable_optional(dev, "hdmi");
+ if (ret != -ENODEV)
+ return ret;
meson_dw_hdmi->hdmitx_apb = devm_reset_control_get_exclusive(dev,
"hdmitx_apb");
--
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 =]
A few regulator consumer drivers seem to be just getting a regulator,
enabling it and registering a devm-action to disable the regulator at
the driver detach and then forget about it.
We can simplify this a bit by adding a devm-helper for this pattern.
Add devm_regulator_get_enable() and devm_regulator_get_enable_optional()
Signed-off-by: Matti Vaittinen <[email protected]>
---
RFCv1 => v2
- Add devm_regulator_bulk_put() and devm_regulator_bulk_get_enable()
---
drivers/regulator/devres.c | 164 +++++++++++++++++++++++++++++
include/linux/regulator/consumer.h | 27 +++++
2 files changed, 191 insertions(+)
diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
index 9113233f41cd..ca3d702b0634 100644
--- a/drivers/regulator/devres.c
+++ b/drivers/regulator/devres.c
@@ -70,6 +70,65 @@ struct regulator *devm_regulator_get_exclusive(struct device *dev,
}
EXPORT_SYMBOL_GPL(devm_regulator_get_exclusive);
+static void regulator_action_disable(void *d)
+{
+ struct regulator *r = (struct regulator *)d;
+
+ regulator_disable(r);
+}
+
+static int _devm_regulator_get_enable(struct device *dev, const char *id,
+ int get_type)
+{
+ struct regulator *r;
+ int ret;
+
+ r = _devm_regulator_get(dev, id, get_type);
+ if (IS_ERR(r))
+ return PTR_ERR(r);
+
+ ret = regulator_enable(r);
+ if (!ret)
+ ret = devm_add_action_or_reset(dev, ®ulator_action_disable, r);
+
+ if (ret)
+ devm_regulator_put(r);
+
+ return ret;
+}
+
+/**
+ * devm_regulator_get_enable_optional - Resource managed regulator get and enable
+ * @dev: device to supply
+ * @id: supply name or regulator ID.
+ *
+ * Get and enable regulator for duration of the device life-time.
+ * regulator_disable() and regulator_put() are automatically called on driver
+ * detach. See regulator_get_optional() and regulator_enable() for more
+ * information.
+ */
+int devm_regulator_get_enable_optional(struct device *dev, const char *id)
+{
+ return _devm_regulator_get_enable(dev, id, OPTIONAL_GET);
+}
+EXPORT_SYMBOL_GPL(devm_regulator_get_enable_optional);
+
+/**
+ * devm_regulator_get_enable - Resource managed regulator get and enable
+ * @dev: device to supply
+ * @id: supply name or regulator ID.
+ *
+ * Get and enable regulator for duration of the device life-time.
+ * regulator_disable() and regulator_put() are automatically called on driver
+ * detach. See regulator_get() and regulator_enable() for more
+ * information.
+ */
+int devm_regulator_get_enable(struct device *dev, const char *id)
+{
+ return _devm_regulator_get_enable(dev, id, NORMAL_GET);
+}
+EXPORT_SYMBOL_GPL(devm_regulator_get_enable);
+
/**
* devm_regulator_get_optional - Resource managed regulator_get_optional()
* @dev: device to supply
@@ -166,6 +225,111 @@ int devm_regulator_bulk_get(struct device *dev, int num_consumers,
}
EXPORT_SYMBOL_GPL(devm_regulator_bulk_get);
+static int devm_regulator_bulk_match(struct device *dev, void *res,
+ void *data)
+{
+ struct regulator_bulk_devres *match = res;
+ struct regulator_bulk_data *target = data;
+
+ /*
+ * We check the put uses same consumer list as the get did.
+ * We _could_ scan all entries in consumer array and check the
+ * regulators match but ATM I don't see the need. We can change this
+ * later if needed.
+ */
+ return match->consumers == target;
+}
+
+/**
+ * devm_regulator_bulk_put - Resource managed regulator_bulk_put()
+ * @consumers: consumers to free
+ *
+ * Deallocate regulators allocated with devm_regulator_bulk_get(). Normally
+ * this function will not need to be called and the resource management
+ * code will ensure that the resource is freed.
+ */
+void devm_regulator_bulk_put(struct regulator_bulk_data *consumers)
+{
+ int rc;
+ struct regulator *regulator = consumers[0].consumer;
+
+ rc = devres_release(regulator->dev, devm_regulator_bulk_release,
+ devm_regulator_bulk_match, consumers);
+ if (rc != 0)
+ WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(devm_regulator_bulk_put);
+
+static void devm_regulator_bulk_disable(void *res)
+{
+ struct regulator_bulk_devres *devres = res;
+ int i;
+
+ for (i = 0; i < devres->num_consumers; i++)
+ regulator_disable(devres->consumers[i].consumer);
+}
+
+/**
+ * devm_regulator_bulk_get_enable - managed get'n enable multiple regulators
+ *
+ * @dev: device to supply
+ * @num_consumers: number of consumers to register
+ * @id: list of supply names or regulator IDs
+ *
+ * @return 0 on success, an errno on failure.
+ *
+ * This helper function allows drivers to get several regulator
+ * consumers in one operation with management, the regulators will
+ * automatically be freed when the device is unbound. If any of the
+ * regulators cannot be acquired then any regulators that were
+ * allocated will be freed before returning to the caller.
+ */
+int devm_regulator_bulk_get_enable(struct device *dev, int num_consumers,
+ const char * const *id)
+{
+ struct regulator_bulk_devres *devres;
+ struct regulator_bulk_data *consumers;
+ int i, ret;
+
+ devres = devm_kmalloc(dev, sizeof(*devres), GFP_KERNEL);
+ if (!devres)
+ return -ENOMEM;
+
+ devres->consumers = devm_kcalloc(dev, num_consumers, sizeof(*consumers),
+ GFP_KERNEL);
+ consumers = devres->consumers;
+ if (!consumers)
+ return -ENOMEM;
+
+ devres->num_consumers = num_consumers;
+
+ for (i = 0; i < num_consumers; i++)
+ consumers[i].supply = id[i];
+
+ ret = devm_regulator_bulk_get(dev, num_consumers, consumers);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < num_consumers; i++) {
+ ret = regulator_enable(consumers[i].consumer);
+ if (ret)
+ goto unwind;
+ }
+
+ ret = devm_add_action(dev, devm_regulator_bulk_disable, devres);
+ if (!ret)
+ return 0;
+
+unwind:
+ while (--i >= 0)
+ regulator_disable(consumers[i].consumer);
+
+ devm_regulator_bulk_put(consumers);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(devm_regulator_bulk_get_enable);
+
static void devm_rdev_release(struct device *dev, void *res)
{
regulator_unregister(*(struct regulator_dev **)res);
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index bbf6590a6dec..24cf5f099eef 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -203,6 +203,8 @@ struct regulator *__must_check regulator_get_optional(struct device *dev,
const char *id);
struct regulator *__must_check devm_regulator_get_optional(struct device *dev,
const char *id);
+int devm_regulator_get_enable(struct device *dev, const char *id);
+int devm_regulator_get_enable_optional(struct device *dev, const char *id);
void regulator_put(struct regulator *regulator);
void devm_regulator_put(struct regulator *regulator);
@@ -240,8 +242,11 @@ int __must_check regulator_bulk_get(struct device *dev, int num_consumers,
struct regulator_bulk_data *consumers);
int __must_check devm_regulator_bulk_get(struct device *dev, int num_consumers,
struct regulator_bulk_data *consumers);
+void devm_regulator_bulk_put(struct regulator_bulk_data *consumers);
int __must_check regulator_bulk_enable(int num_consumers,
struct regulator_bulk_data *consumers);
+int devm_regulator_bulk_get_enable(struct device *dev, int num_consumers,
+ const char * const *id);
int regulator_bulk_disable(int num_consumers,
struct regulator_bulk_data *consumers);
int regulator_bulk_force_disable(int num_consumers,
@@ -346,6 +351,17 @@ devm_regulator_get_exclusive(struct device *dev, const char *id)
return ERR_PTR(-ENODEV);
}
+static inline int devm_regulator_get_enable(struct device *dev, const char *id)
+{
+ return -ENODEV;
+}
+
+static inline int devm_regulator_get_enable_optional(struct device *dev,
+ const char *id)
+{
+ return -ENODEV;
+}
+
static inline struct regulator *__must_check
regulator_get_optional(struct device *dev, const char *id)
{
@@ -367,6 +383,10 @@ static inline void devm_regulator_put(struct regulator *regulator)
{
}
+static inline void devm_regulator_bulk_put(struct regulator_bulk_data *consumers)
+{
+}
+
static inline int regulator_register_supply_alias(struct device *dev,
const char *id,
struct device *alias_dev,
@@ -457,6 +477,13 @@ static inline int regulator_bulk_enable(int num_consumers,
return 0;
}
+static inline int devm_regulator_bulk_get_enable(struct device *dev,
+ int num_consumers,
+ const char * const *id)
+{
+ return 0;
+}
+
static inline int regulator_bulk_disable(int num_consumers,
struct regulator_bulk_data *consumers)
{
--
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 =]
Simplify the driver using devm_regulator_get_enable() instead of
open-coding the devm_add_action_or_reset().
A (minor?) functional change is that we don't print an error in case of a
deferred probe. Now we also print the error no matter which of the
involved calls caused the failure.
Signed-off-by: Matti Vaittinen <[email protected]>
---
RFCv1 => v2:
- No changes
---
drivers/clk/clk-cdce925.c | 21 ++++-----------------
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/drivers/clk/clk-cdce925.c b/drivers/clk/clk-cdce925.c
index ef9a2d44e40c..6350682f7e6d 100644
--- a/drivers/clk/clk-cdce925.c
+++ b/drivers/clk/clk-cdce925.c
@@ -603,28 +603,15 @@ of_clk_cdce925_get(struct of_phandle_args *clkspec, void *_data)
return &data->clk[idx].hw;
}
-static void cdce925_regulator_disable(void *regulator)
-{
- regulator_disable(regulator);
-}
-
static int cdce925_regulator_enable(struct device *dev, const char *name)
{
- struct regulator *regulator;
int err;
- regulator = devm_regulator_get(dev, name);
- if (IS_ERR(regulator))
- return PTR_ERR(regulator);
-
- err = regulator_enable(regulator);
- if (err) {
- dev_err(dev, "Failed to enable %s: %d\n", name, err);
- return err;
- }
+ err = devm_regulator_get_enable(dev, name);
+ if (err)
+ dev_err_probe(dev, err, "Failed to enable %s:\n", name);
- return devm_add_action_or_reset(dev, cdce925_regulator_disable,
- regulator);
+ return err;
}
/* The CDCE925 uses a funky way to read/write registers. Bulk mode is
--
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 =]
A few managed regulator functions were missing from the API list.
Add missing functions.
Signed-off-by: Matti Vaittinen <[email protected]>
---
RFCv1 => v2:
- No changes
This one is actually a documentation fix which adds existing APIs to the
list. I guess this patch is good for being merged independently from the
rest of the series.
---
Documentation/driver-api/driver-model/devres.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
index 2d39967bafcc..271d1eb2234b 100644
--- a/Documentation/driver-api/driver-model/devres.rst
+++ b/Documentation/driver-api/driver-model/devres.rst
@@ -406,10 +406,17 @@ PWM
devm_fwnode_pwm_get()
REGULATOR
+ devm_regulator_bulk_register_supply_alias()
devm_regulator_bulk_get()
devm_regulator_get()
+ devm_regulator_get_exclusive()
+ devm_regulator_get_optional()
+ devm_regulator_irq_helper()
devm_regulator_put()
devm_regulator_register()
+ devm_regulator_register_notifier()
+ devm_regulator_register_supply_alias()
+ devm_regulator_unregister_notifier()
RESET
devm_reset_control_get()
--
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 =]
Morning Andy,
On Fri, Aug 12, 2022 at 09:05:52PM +0200, Andy Shevchenko wrote:
> On Friday, August 12, 2022, Matti Vaittinen <[email protected]>
> wrote:
>
> > adc/ad7192:
> > Use devm_regulator_get_enable() instead of open coded get, enable,
> > add-action-to-disable-at-detach - pattern. Also drop the seemingly unused
> > struct member 'dvdd'.
>
> In IIO we expect to have one patch per driver. Please split.
Fine with me. I didn't go through too many drivers anyways. I'll split
for the v3. I'll just wait for a while for other feedback.
Best Regards
Matti
--
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 =]