[PATCH v3 00/14] Use devm helpers for regulator get and enable

[PATCH v3 00/14] Use devm helpers for regulator get and enable

[Matti Vaittinen]

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

Use devm helpers for regulator get and enable

NOTE: The series depends on commit
ee94aff2628b ("Devm helpers for regulator get and enable")
which currently sits in Mark's regulator/for-next

A few* drivers seem to pattern demonstrated by pseudocode:

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

devm helpers for this pattern were added to 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.

I believe this simplifies things by removing some dublicated code.

This series reowrks a few drivers. 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:

v3:
	- Drop already applied helper patches
	- Add a few more drivers

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:
	Add new devm-helper APIs to docs.
Patch 2:
	simplified CLK driver(s)
Patch 3:
	simplified GPU driver(s)
Patch 4 - 5:
	simplified hwmon driver(s)
Patch 6 - 14:
	simplified IIO driver(s)

---

Matti Vaittinen (14):
  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()
  hwmon: adm1177: simplify using devm_regulator_get_enable()
  iio: ad7192: Simplify using devm_regulator_get_enable()
  iio: ltc2688: Simplify using devm_regulator_*get_enable()
  iio: bmg160_core: Simplify using devm_regulator_*get_enable()
  iio: st_lsm6dsx: Simplify using devm_regulator_*get_enable()
  iio: ad7476: simplify using devm_regulator_get_enable()
  iio: ad7606: simplify using devm_regulator_get_enable()
  iio: max1241: simplify using devm_regulator_get_enable()
  iio: max1363: simplify using devm_regulator_get_enable()
  iio: hmc425a: simplify using devm_regulator_get_enable()

 .../driver-api/driver-model/devres.rst        |  4 +++
 drivers/clk/clk-cdce925.c                     | 21 +++----------
 drivers/gpu/drm/bridge/sii902x.c              | 22 ++------------
 drivers/gpu/drm/meson/meson_dw_hdmi.c         | 23 ++------------
 drivers/hwmon/adm1177.c                       | 27 ++---------------
 drivers/hwmon/lm90.c                          | 15 ++--------
 drivers/iio/adc/ad7192.c                      | 15 ++--------
 drivers/iio/adc/ad7476.c                      | 11 +------
 drivers/iio/adc/ad7606.c                      | 22 ++------------
 drivers/iio/adc/ad7606.h                      |  1 -
 drivers/iio/adc/max1241.c                     | 28 ++---------------
 drivers/iio/adc/max1363.c                     | 11 +------
 drivers/iio/amplifiers/hmc425a.c              | 17 +----------
 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 ++++---------------
 17 files changed, 41 insertions(+), 255 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 --]

[PATCH v3 04/14] hwmon: lm90: simplify using devm_regulator_get_enable()

[Matti Vaittinen]

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

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 <mazziesaccount@gmail.com>
Acked-by: Guenter Roeck <linux@roeck-us.net>

---
RFCv1 => onwards:
- No changes
---
 drivers/hwmon/lm90.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index 221de01a327a..6356d92547e0 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -2749,24 +2749,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 =] 

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

[PATCH v3 05/14] hwmon: adm1177: simplify using devm_regulator_get_enable()

[Matti Vaittinen]

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

Drop open-coded pattern: 'devm_regulator_get(), regulator_enable(),
add_action_or_reset(regulator_disable)' and use the
devm_regulator_get_enable() and drop the pointer to the regulator.
This simplifies code and makes it less tempting to add manual control
for the regulator which is also controlled by devm.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

---
v2 => v3:
New patch
---
 drivers/hwmon/adm1177.c | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

diff --git a/drivers/hwmon/adm1177.c b/drivers/hwmon/adm1177.c
index 0c5dbc5e33b4..be17a26a84f1 100644
--- a/drivers/hwmon/adm1177.c
+++ b/drivers/hwmon/adm1177.c
@@ -26,14 +26,12 @@
 /**
  * struct adm1177_state - driver instance specific data
  * @client:		pointer to i2c client
- * @reg:		regulator info for the power supply of the device
  * @r_sense_uohm:	current sense resistor value
  * @alert_threshold_ua:	current limit for shutdown
  * @vrange_high:	internal voltage divider
  */
 struct adm1177_state {
 	struct i2c_client	*client;
-	struct regulator	*reg;
 	u32			r_sense_uohm;
 	u32			alert_threshold_ua;
 	bool			vrange_high;
@@ -189,13 +187,6 @@ static const struct hwmon_chip_info adm1177_chip_info = {
 	.info = adm1177_info,
 };
 
-static void adm1177_remove(void *data)
-{
-	struct adm1177_state *st = data;
-
-	regulator_disable(st->reg);
-}
-
 static int adm1177_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -210,21 +201,9 @@ static int adm1177_probe(struct i2c_client *client)
 
 	st->client = client;
 
-	st->reg = devm_regulator_get_optional(&client->dev, "vref");
-	if (IS_ERR(st->reg)) {
-		if (PTR_ERR(st->reg) == -EPROBE_DEFER)
-			return -EPROBE_DEFER;
-
-		st->reg = NULL;
-	} else {
-		ret = regulator_enable(st->reg);
-		if (ret)
-			return ret;
-		ret = devm_add_action_or_reset(&client->dev, adm1177_remove,
-					       st);
-		if (ret)
-			return ret;
-	}
+	ret = devm_regulator_get_enable_optional(&client->dev, "vref");
+	if (ret == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
 
 	if (device_property_read_u32(dev, "shunt-resistor-micro-ohms",
 				     &st->r_sense_uohm))
-- 
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 --]

Re: [PATCH v3 05/14] hwmon: adm1177: simplify using devm_regulator_get_enable()

[Guenter Roeck]

On Fri, Aug 19, 2022 at 10:18:46PM +0300, Matti Vaittinen wrote:
> Drop open-coded pattern: 'devm_regulator_get(), regulator_enable(),
> add_action_or_reset(regulator_disable)' and use the
> devm_regulator_get_enable() and drop the pointer to the regulator.
> This simplifies code and makes it less tempting to add manual control
> for the regulator which is also controlled by devm.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

Acked-by: Guenter Roeck <linux@roeck-us.net>

> 
> ---
> v2 => v3:
> New patch
> ---
>  drivers/hwmon/adm1177.c | 27 +++------------------------
>  1 file changed, 3 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/hwmon/adm1177.c b/drivers/hwmon/adm1177.c
> index 0c5dbc5e33b4..be17a26a84f1 100644
> --- a/drivers/hwmon/adm1177.c
> +++ b/drivers/hwmon/adm1177.c
> @@ -26,14 +26,12 @@
>  /**
>   * struct adm1177_state - driver instance specific data
>   * @client:		pointer to i2c client
> - * @reg:		regulator info for the power supply of the device
>   * @r_sense_uohm:	current sense resistor value
>   * @alert_threshold_ua:	current limit for shutdown
>   * @vrange_high:	internal voltage divider
>   */
>  struct adm1177_state {
>  	struct i2c_client	*client;
> -	struct regulator	*reg;
>  	u32			r_sense_uohm;
>  	u32			alert_threshold_ua;
>  	bool			vrange_high;
> @@ -189,13 +187,6 @@ static const struct hwmon_chip_info adm1177_chip_info = {
>  	.info = adm1177_info,
>  };
>  
> -static void adm1177_remove(void *data)
> -{
> -	struct adm1177_state *st = data;
> -
> -	regulator_disable(st->reg);
> -}
> -
>  static int adm1177_probe(struct i2c_client *client)
>  {
>  	struct device *dev = &client->dev;
> @@ -210,21 +201,9 @@ static int adm1177_probe(struct i2c_client *client)
>  
>  	st->client = client;
>  
> -	st->reg = devm_regulator_get_optional(&client->dev, "vref");
> -	if (IS_ERR(st->reg)) {
> -		if (PTR_ERR(st->reg) == -EPROBE_DEFER)
> -			return -EPROBE_DEFER;
> -
> -		st->reg = NULL;
> -	} else {
> -		ret = regulator_enable(st->reg);
> -		if (ret)
> -			return ret;
> -		ret = devm_add_action_or_reset(&client->dev, adm1177_remove,
> -					       st);
> -		if (ret)
> -			return ret;
> -	}
> +	ret = devm_regulator_get_enable_optional(&client->dev, "vref");
> +	if (ret == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;
>  
>  	if (device_property_read_u32(dev, "shunt-resistor-micro-ohms",
>  				     &st->r_sense_uohm))
> -- 
> 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 =] 

Re: [PATCH v3 00/14] Use devm helpers for regulator get and enable

[Andy Shevchenko]

On Fri, Aug 19, 2022 at 10:20 PM Matti Vaittinen
<mazziesaccount@gmail.com> wrote:
>
> Use devm helpers for regulator get and enable
>
> NOTE: The series depends on commit
> ee94aff2628b ("Devm helpers for regulator get and enable")
> which currently sits in Mark's regulator/for-next
>
> A few* drivers seem to pattern demonstrated by pseudocode:
>
> - devm_regulator_get()
> - regulator_enable()
> - devm_add_action_or_reset(regulator_disable())
>
> devm helpers for this pattern were added to remove bunch of code from

remove a bunch

> 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.
>
> I believe this simplifies things by removing some dublicated code.

duplicated

> This series reowrks a few drivers. There is still plenty of fish in the

reworks

> 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!).

...

>   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()
>   hwmon: adm1177: simplify using devm_regulator_get_enable()

hwmon uses a different pattern for the Subject line.

-- 
With Best Regards,
Andy Shevchenko