[PATCH 1/7] regulator: anatop-regulator: Use devm_regulator_register

[PATCH 1/7] regulator: anatop-regulator: Use devm_regulator_register

[Sachin Kamat]

devm_* simplifies the code.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
This series is compile tested.
---
 drivers/regulator/anatop-regulator.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 0d4a8cc..e42bfd1 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -200,7 +200,7 @@ static int anatop_regulator_probe(struct platform_device *pdev)
 	config.regmap = sreg->anatop;
 
 	/* register regulator */
-	rdev = regulator_register(rdesc, &config);
+	rdev = devm_regulator_register(dev, rdesc, &config);
 	if (IS_ERR(rdev)) {
 		dev_err(dev, "failed to register %s\n",
 			rdesc->name);
@@ -223,7 +223,6 @@ static int anatop_regulator_remove(struct platform_device *pdev)
 	struct anatop_regulator *sreg = rdev_get_drvdata(rdev);
 	const char *name = sreg->name;
 
-	regulator_unregister(rdev);
 	kfree(name);
 
 	return 0;
-- 
1.7.9.5

[PATCH 2/7] regulator: isl6271a-regulator: Use devm_regulator_register

[Sachin Kamat]

devm_* simplifies the code.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/regulator/isl6271a-regulator.c |   24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/regulator/isl6271a-regulator.c b/drivers/regulator/isl6271a-regulator.c
index b99c49b..40c224f 100644
--- a/drivers/regulator/isl6271a-regulator.c
+++ b/drivers/regulator/isl6271a-regulator.c
@@ -112,7 +112,7 @@ static int isl6271a_probe(struct i2c_client *i2c,
 	struct regulator_config config = { };
 	struct regulator_init_data *init_data	= i2c->dev.platform_data;
 	struct isl_pmic *pmic;
-	int err, i;
+	int i;
 
 	if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -EIO;
@@ -133,32 +133,17 @@ static int isl6271a_probe(struct i2c_client *i2c,
 			config.init_data = NULL;
 		config.driver_data = pmic;
 
-		pmic->rdev[i] = regulator_register(&isl_rd[i], &config);
+		pmic->rdev[i] = devm_regulator_register(&i2c->dev, &isl_rd[i],
+							&config);
 		if (IS_ERR(pmic->rdev[i])) {
 			dev_err(&i2c->dev, "failed to register %s\n", id->name);
-			err = PTR_ERR(pmic->rdev[i]);
-			goto error;
+			return PTR_ERR(pmic->rdev[i]);
 		}
 	}
 
 	i2c_set_clientdata(i2c, pmic);
 
 	return 0;
-
-error:
-	while (--i >= 0)
-		regulator_unregister(pmic->rdev[i]);
-	return err;
-}
-
-static int isl6271a_remove(struct i2c_client *i2c)
-{
-	struct isl_pmic *pmic = i2c_get_clientdata(i2c);
-	int i;
-
-	for (i = 0; i < 3; i++)
-		regulator_unregister(pmic->rdev[i]);
-	return 0;
 }
 
 static const struct i2c_device_id isl6271a_id[] = {
@@ -174,7 +159,6 @@ static struct i2c_driver isl6271a_i2c_driver = {
 		.owner = THIS_MODULE,
 	},
 	.probe = isl6271a_probe,
-	.remove = isl6271a_remove,
 	.id_table = isl6271a_id,
 };
 
-- 
1.7.9.5

Re: [PATCH 2/7] regulator: isl6271a-regulator: Use devm_regulator_register

[Marek Vasut]

Dear Sachin Kamat,

> devm_* simplifies the code.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>

Acked-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[PATCH 3/7] regulator: mc13783: Use devm_regulator_register

[Sachin Kamat]

devm_* simplifies the code.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/regulator/mc13783-regulator.c |   25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/regulator/mc13783-regulator.c b/drivers/regulator/mc13783-regulator.c
index 5ff99d2..f036b26 100644
--- a/drivers/regulator/mc13783-regulator.c
+++ b/drivers/regulator/mc13783-regulator.c
@@ -400,7 +400,7 @@ static int mc13783_regulator_probe(struct platform_device *pdev)
 		dev_get_platdata(&pdev->dev);
 	struct mc13xxx_regulator_init_data *mc13xxx_data;
 	struct regulator_config config = { };
-	int i, ret, num_regulators;
+	int i, num_regulators;
 
 	num_regulators = mc13xxx_get_num_regulators_dt(pdev);
 
@@ -444,32 +444,16 @@ static int mc13783_regulator_probe(struct platform_device *pdev)
 		config.driver_data = priv;
 		config.of_node = node;
 
-		priv->regulators[i] = regulator_register(desc, &config);
+		priv->regulators[i] = devm_regulator_register(&pdev->dev, desc,
+							      &config);
 		if (IS_ERR(priv->regulators[i])) {
 			dev_err(&pdev->dev, "failed to register regulator %s\n",
 				mc13783_regulators[i].desc.name);
-			ret = PTR_ERR(priv->regulators[i]);
-			goto err;
+			return PTR_ERR(priv->regulators[i]);
 		}
 	}
 
 	return 0;
-err:
-	while (--i >= 0)
-		regulator_unregister(priv->regulators[i]);
-
-	return ret;
-}
-
-static int mc13783_regulator_remove(struct platform_device *pdev)
-{
-	struct mc13xxx_regulator_priv *priv = platform_get_drvdata(pdev);
-	int i;
-
-	for (i = 0; i < priv->num_regulators; i++)
-		regulator_unregister(priv->regulators[i]);
-
-	return 0;
 }
 
 static struct platform_driver mc13783_regulator_driver = {
@@ -477,7 +461,6 @@ static struct platform_driver mc13783_regulator_driver = {
 		.name	= "mc13783-regulator",
 		.owner	= THIS_MODULE,
 	},
-	.remove		= mc13783_regulator_remove,
 	.probe		= mc13783_regulator_probe,
 };
 
-- 
1.7.9.5

Re: [PATCH 3/7] regulator: mc13783: Use devm_regulator_register

[Marek Vasut]

Dear Sachin Kamat,

> devm_* simplifies the code.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>

Acked-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[PATCH 4/7] regulator: mc13892: Use devm_regulator_register

[Sachin Kamat]

devm_* simplifies the code.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/regulator/mc13892-regulator.c |   22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/regulator/mc13892-regulator.c b/drivers/regulator/mc13892-regulator.c
index 1037e07..96c9f80 100644
--- a/drivers/regulator/mc13892-regulator.c
+++ b/drivers/regulator/mc13892-regulator.c
@@ -611,43 +611,27 @@ static int mc13892_regulator_probe(struct platform_device *pdev)
 		config.driver_data = priv;
 		config.of_node = node;
 
-		priv->regulators[i] = regulator_register(desc, &config);
+		priv->regulators[i] = devm_regulator_register(&pdev->dev, desc,
+							      &config);
 		if (IS_ERR(priv->regulators[i])) {
 			dev_err(&pdev->dev, "failed to register regulator %s\n",
 				mc13892_regulators[i].desc.name);
-			ret = PTR_ERR(priv->regulators[i]);
-			goto err;
+			return PTR_ERR(priv->regulators[i]);
 		}
 	}
 
 	return 0;
-err:
-	while (--i >= 0)
-		regulator_unregister(priv->regulators[i]);
-	return ret;
 
 err_unlock:
 	mc13xxx_unlock(mc13892);
 	return ret;
 }
 
-static int mc13892_regulator_remove(struct platform_device *pdev)
-{
-	struct mc13xxx_regulator_priv *priv = platform_get_drvdata(pdev);
-	int i;
-
-	for (i = 0; i < priv->num_regulators; i++)
-		regulator_unregister(priv->regulators[i]);
-
-	return 0;
-}
-
 static struct platform_driver mc13892_regulator_driver = {
 	.driver	= {
 		.name	= "mc13892-regulator",
 		.owner	= THIS_MODULE,
 	},
-	.remove	= mc13892_regulator_remove,
 	.probe	= mc13892_regulator_probe,
 };
 
-- 
1.7.9.5

Re: [PATCH 4/7] regulator: mc13892: Use devm_regulator_register

[Marek Vasut]

Dear Sachin Kamat,

> devm_* simplifies the code.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>

Acked-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[PATCH 5/7] regulator: palmas: Use devm_regulator_register

[Sachin Kamat]

devm_* simplifies the code.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/regulator/palmas-regulator.c |   38 ++++++++++------------------------
 1 file changed, 11 insertions(+), 27 deletions(-)

diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
index d0c8785..37b1068 100644
--- a/drivers/regulator/palmas-regulator.c
+++ b/drivers/regulator/palmas-regulator.c
@@ -852,7 +852,7 @@ static int palmas_regulators_probe(struct platform_device *pdev)
 			if (ret < 0) {
 				dev_err(&pdev->dev,
 					"reading TSTEP reg failed: %d\n", ret);
-				goto err_unregister_regulator;
+				return ret;
 			}
 			pmic->desc[id].ramp_delay =
 					palmas_smps_ramp_delay[reg & 0x3];
@@ -864,7 +864,7 @@ static int palmas_regulators_probe(struct platform_device *pdev)
 			reg_init = pdata->reg_init[id];
 			ret = palmas_smps_init(palmas, id, reg_init);
 			if (ret)
-				goto err_unregister_regulator;
+				return ret;
 		}
 
 		/* Register the regulators */
@@ -897,7 +897,7 @@ static int palmas_regulators_probe(struct platform_device *pdev)
 
 			ret = palmas_smps_read(pmic->palmas, addr, &reg);
 			if (ret)
-				goto err_unregister_regulator;
+				return ret;
 			if (reg & PALMAS_SMPS12_VOLTAGE_RANGE)
 				pmic->range[id] = 1;
 
@@ -913,7 +913,7 @@ static int palmas_regulators_probe(struct platform_device *pdev)
 			addr = palmas_regs_info[id].ctrl_addr;
 			ret = palmas_smps_read(pmic->palmas, addr, &reg);
 			if (ret)
-				goto err_unregister_regulator;
+				return ret;
 			pmic->current_reg_mode[id] = reg &
 					PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK;
 		}
@@ -929,13 +929,13 @@ static int palmas_regulators_probe(struct platform_device *pdev)
 		pmic->desc[id].supply_name = palmas_regs_info[id].sname;
 		config.of_node = palmas_matches[id].of_node;
 
-		rdev = regulator_register(&pmic->desc[id], &config);
+		rdev = devm_regulator_register(&pdev->dev, &pmic->desc[id],
+					       &config);
 		if (IS_ERR(rdev)) {
 			dev_err(&pdev->dev,
 				"failed to register %s regulator\n",
 				pdev->name);
-			ret = PTR_ERR(rdev);
-			goto err_unregister_regulator;
+			return PTR_ERR(rdev);
 		}
 
 		/* Save regulator for cleanup */
@@ -997,13 +997,13 @@ static int palmas_regulators_probe(struct platform_device *pdev)
 		pmic->desc[id].supply_name = palmas_regs_info[id].sname;
 		config.of_node = palmas_matches[id].of_node;
 
-		rdev = regulator_register(&pmic->desc[id], &config);
+		rdev = devm_regulator_register(&pdev->dev, &pmic->desc[id],
+					       &config);
 		if (IS_ERR(rdev)) {
 			dev_err(&pdev->dev,
 				"failed to register %s regulator\n",
 				pdev->name);
-			ret = PTR_ERR(rdev);
-			goto err_unregister_regulator;
+			return PTR_ERR(rdev);
 		}
 
 		/* Save regulator for cleanup */
@@ -1021,7 +1021,7 @@ static int palmas_regulators_probe(struct platform_device *pdev)
 							id, reg_init);
 				if (ret) {
 					regulator_unregister(pmic->rdev[id]);
-					goto err_unregister_regulator;
+					return ret;
 				}
 			}
 		}
@@ -1029,21 +1029,6 @@ static int palmas_regulators_probe(struct platform_device *pdev)
 
 
 	return 0;
-
-err_unregister_regulator:
-	while (--id >= 0)
-		regulator_unregister(pmic->rdev[id]);
-	return ret;
-}
-
-static int palmas_regulators_remove(struct platform_device *pdev)
-{
-	struct palmas_pmic *pmic = platform_get_drvdata(pdev);
-	int id;
-
-	for (id = 0; id < PALMAS_NUM_REGS; id++)
-		regulator_unregister(pmic->rdev[id]);
-	return 0;
 }
 
 static struct of_device_id of_palmas_match_tbl[] = {
@@ -1065,7 +1050,6 @@ static struct platform_driver palmas_driver = {
 		.owner = THIS_MODULE,
 	},
 	.probe = palmas_regulators_probe,
-	.remove = palmas_regulators_remove,
 };
 
 static int __init palmas_init(void)
-- 
1.7.9.5

Re: [PATCH 5/7] regulator: palmas: Use devm_regulator_register

[Marek Vasut]

Dear Sachin Kamat,

> devm_* simplifies the code.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  drivers/regulator/palmas-regulator.c |   38
> ++++++++++------------------------ 1 file changed, 11 insertions(+), 27
> deletions(-)
> 
> diff --git a/drivers/regulator/palmas-regulator.c
> b/drivers/regulator/palmas-regulator.c index d0c8785..37b1068 100644
> --- a/drivers/regulator/palmas-regulator.c
> +++ b/drivers/regulator/palmas-regulator.c
> @@ -852,7 +852,7 @@ static int palmas_regulators_probe(struct
> platform_device *pdev) if (ret < 0) {
>  				dev_err(&pdev->dev,
>  					"reading TSTEP reg failed: %d\n", ret);
> -				goto err_unregister_regulator;
> +				return ret;
>  			}
>  			pmic->desc[id].ramp_delay =
>  					palmas_smps_ramp_delay[reg & 0x3];
> @@ -864,7 +864,7 @@ static int palmas_regulators_probe(struct
> platform_device *pdev) reg_init = pdata->reg_init[id];
>  			ret = palmas_smps_init(palmas, id, reg_init);
>  			if (ret)
> -				goto err_unregister_regulator;
> +				return ret;
>  		}
> 
>  		/* Register the regulators */
> @@ -897,7 +897,7 @@ static int palmas_regulators_probe(struct
> platform_device *pdev)
> 
>  			ret = palmas_smps_read(pmic->palmas, addr, &reg);
>  			if (ret)
> -				goto err_unregister_regulator;
> +				return ret;
>  			if (reg & PALMAS_SMPS12_VOLTAGE_RANGE)
>  				pmic->range[id] = 1;
> 
> @@ -913,7 +913,7 @@ static int palmas_regulators_probe(struct
> platform_device *pdev) addr = palmas_regs_info[id].ctrl_addr;
>  			ret = palmas_smps_read(pmic->palmas, addr, &reg);
>  			if (ret)
> -				goto err_unregister_regulator;
> +				return ret;
>  			pmic->current_reg_mode[id] = reg &
>  					PALMAS_SMPS12_CTRL_MODE_ACTIVE_MASK;
>  		}
> @@ -929,13 +929,13 @@ static int palmas_regulators_probe(struct
> platform_device *pdev) pmic->desc[id].supply_name =
> palmas_regs_info[id].sname;
>  		config.of_node = palmas_matches[id].of_node;
> 
> -		rdev = regulator_register(&pmic->desc[id], &config);
> +		rdev = devm_regulator_register(&pdev->dev, &pmic->desc[id],
> +					       &config);
>  		if (IS_ERR(rdev)) {
>  			dev_err(&pdev->dev,
>  				"failed to register %s regulator\n",
>  				pdev->name);
> -			ret = PTR_ERR(rdev);
> -			goto err_unregister_regulator;
> +			return PTR_ERR(rdev);
>  		}
> 
>  		/* Save regulator for cleanup */
> @@ -997,13 +997,13 @@ static int palmas_regulators_probe(struct
> platform_device *pdev) pmic->desc[id].supply_name =
> palmas_regs_info[id].sname;
>  		config.of_node = palmas_matches[id].of_node;
> 
> -		rdev = regulator_register(&pmic->desc[id], &config);
> +		rdev = devm_regulator_register(&pdev->dev, &pmic->desc[id],
> +					       &config);
>  		if (IS_ERR(rdev)) {
>  			dev_err(&pdev->dev,
>  				"failed to register %s regulator\n",
>  				pdev->name);
> -			ret = PTR_ERR(rdev);
> -			goto err_unregister_regulator;
> +			return PTR_ERR(rdev);
>  		}
> 
>  		/* Save regulator for cleanup */
> @@ -1021,7 +1021,7 @@ static int palmas_regulators_probe(struct
> platform_device *pdev) id, reg_init);
>  				if (ret) {
>  					regulator_unregister(pmic->rdev[id]);
> -					goto err_unregister_regulator;
> +					return ret;
>  				}

what about this regulator_unregister() above here?

[...]
Best regards,
Marek Vasut

Re: [PATCH 5/7] regulator: palmas: Use devm_regulator_register

[Sachin Kamat]

On 4 September 2013 20:02, Marek Vasut <marex@denx.de> wrote:
> Dear Sachin Kamat,
>
>> devm_* simplifies the code.
>>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> ---
>>  drivers/regulator/palmas-regulator.c |   38
>> @@ -1021,7 +1021,7 @@ static int palmas_regulators_probe(struct
>> platform_device *pdev) id, reg_init);
>>                               if (ret) {
>>                                       regulator_unregister(pmic->rdev[id]);
>> -                                     goto err_unregister_regulator;
>> +                                     return ret;
>>                               }
>
> what about this regulator_unregister() above here?

It should have gone too. I will send a fix for that. Thanks for noticing.



-- 
With warm regards,
Sachin

[PATCH 6/7] regulator: rc5t583: Use devm_regulator_register

[Sachin Kamat]

devm_* simplifies the code.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/regulator/rc5t583-regulator.c |   22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/regulator/rc5t583-regulator.c b/drivers/regulator/rc5t583-regulator.c
index 5885b45..b58affb 100644
--- a/drivers/regulator/rc5t583-regulator.c
+++ b/drivers/regulator/rc5t583-regulator.c
@@ -173,33 +173,16 @@ skip_ext_pwr_config:
 		config.driver_data = reg;
 		config.regmap = rc5t583->regmap;
 
-		rdev = regulator_register(&ri->desc, &config);
+		rdev = devm_regulator_register(&pdev->dev, &ri->desc, &config);
 		if (IS_ERR(rdev)) {
 			dev_err(&pdev->dev, "Failed to register regulator %s\n",
 						ri->desc.name);
-			ret = PTR_ERR(rdev);
-			goto clean_exit;
+			return PTR_ERR(rdev);
 		}
 		reg->rdev = rdev;
 	}
 	platform_set_drvdata(pdev, regs);
 	return 0;
-
-clean_exit:
-	while (--id >= 0)
-		regulator_unregister(regs[id].rdev);
-
-	return ret;
-}
-
-static int rc5t583_regulator_remove(struct platform_device *pdev)
-{
-	struct rc5t583_regulator *regs = platform_get_drvdata(pdev);
-	int id;
-
-	for (id = 0; id < RC5T583_REGULATOR_MAX; ++id)
-		regulator_unregister(regs[id].rdev);
-	return 0;
 }
 
 static struct platform_driver rc5t583_regulator_driver = {
@@ -208,7 +191,6 @@ static struct platform_driver rc5t583_regulator_driver = {
 		.owner	= THIS_MODULE,
 	},
 	.probe		= rc5t583_regulator_probe,
-	.remove		= rc5t583_regulator_remove,
 };
 
 static int __init rc5t583_regulator_init(void)
-- 
1.7.9.5

Re: [PATCH 6/7] regulator: rc5t583: Use devm_regulator_register

[Marek Vasut]

Dear Sachin Kamat,

> devm_* simplifies the code.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>  drivers/regulator/rc5t583-regulator.c |   22 ++--------------------
>  1 file changed, 2 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/regulator/rc5t583-regulator.c
> b/drivers/regulator/rc5t583-regulator.c index 5885b45..b58affb 100644
> --- a/drivers/regulator/rc5t583-regulator.c
> +++ b/drivers/regulator/rc5t583-regulator.c
> @@ -173,33 +173,16 @@ skip_ext_pwr_config:
>  		config.driver_data = reg;
>  		config.regmap = rc5t583->regmap;
> 
> -		rdev = regulator_register(&ri->desc, &config);
> +		rdev = devm_regulator_register(&pdev->dev, &ri->desc, &config);
>  		if (IS_ERR(rdev)) {
>  			dev_err(&pdev->dev, "Failed to register regulator %s\n",
>  						ri->desc.name);
> -			ret = PTR_ERR(rdev);
> -			goto clean_exit;
> +			return PTR_ERR(rdev);
>  		}
>  		reg->rdev = rdev;
>  	}
>  	platform_set_drvdata(pdev, regs);
>  	return 0;
> -
> -clean_exit:
> -	while (--id >= 0)
> -		regulator_unregister(regs[id].rdev);
> -
> -	return ret;
> -}
> -
> -static int rc5t583_regulator_remove(struct platform_device *pdev)
> -{
> -	struct rc5t583_regulator *regs = platform_get_drvdata(pdev);
> -	int id;
> -
> -	for (id = 0; id < RC5T583_REGULATOR_MAX; ++id)
> -		regulator_unregister(regs[id].rdev);
> -	return 0;
>  }
> 
>  static struct platform_driver rc5t583_regulator_driver = {
> @@ -208,7 +191,6 @@ static struct platform_driver rc5t583_regulator_driver
> = { .owner	= THIS_MODULE,
>  	},
>  	.probe		= rc5t583_regulator_probe,
> -	.remove		= rc5t583_regulator_remove,
>  };
> 
>  static int __init rc5t583_regulator_init(void)

Next up, you might want to start flipping the regulator drivers to 
module_platform_driver ;-)

Acked-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

[PATCH 7/7] regulator: ti-abb: Use devm_regulator_register

[Sachin Kamat]

devm_* simplifies the code.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
 drivers/regulator/ti-abb-regulator.c |   82 ++++++++++------------------------
 1 file changed, 23 insertions(+), 59 deletions(-)

diff --git a/drivers/regulator/ti-abb-regulator.c b/drivers/regulator/ti-abb-regulator.c
index 3753ed0..02392f8 100644
--- a/drivers/regulator/ti-abb-regulator.c
+++ b/drivers/regulator/ti-abb-regulator.c
@@ -696,22 +696,17 @@ static int ti_abb_probe(struct platform_device *pdev)
 	match = of_match_device(ti_abb_of_match, dev);
 	if (!match) {
 		/* We do not expect this to happen */
-		ret = -ENODEV;
 		dev_err(dev, "%s: Unable to match device\n", __func__);
-		goto err;
+		return -ENODEV;
 	}
 	if (!match->data) {
-		ret = -EINVAL;
 		dev_err(dev, "%s: Bad data in match\n", __func__);
-		goto err;
+		return -EINVAL;
 	}
 
 	abb = devm_kzalloc(dev, sizeof(struct ti_abb), GFP_KERNEL);
-	if (!abb) {
-		dev_err(dev, "%s: Unable to allocate ABB struct\n", __func__);
-		ret = -ENOMEM;
-		goto err;
-	}
+	if (!abb)
+		return -ENOMEM;
 	abb->regs = match->data;
 
 	/* Map ABB resources */
@@ -719,21 +714,17 @@ static int ti_abb_probe(struct platform_device *pdev)
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
 	if (!res) {
 		dev_err(dev, "Missing '%s' IO resource\n", pname);
-		ret = -ENODEV;
-		goto err;
+		return -ENODEV;
 	}
 	abb->base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(abb->base)) {
-		ret = PTR_ERR(abb->base);
-		goto err;
-	}
+	if (IS_ERR(abb->base))
+		return PTR_ERR(abb->base);
 
 	pname = "int-address";
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
 	if (!res) {
 		dev_err(dev, "Missing '%s' IO resource\n", pname);
-		ret = -ENODEV;
-		goto err;
+		return -ENODEV;
 	}
 	/*
 	 * We may have shared interrupt register offsets which are
@@ -743,8 +734,7 @@ static int ti_abb_probe(struct platform_device *pdev)
 					     resource_size(res));
 	if (!abb->int_base) {
 		dev_err(dev, "Unable to map '%s'\n", pname);
-		ret = -ENOMEM;
-		goto err;
+		return -ENOMEM;
 	}
 
 	/* Map Optional resources */
@@ -764,8 +754,7 @@ static int ti_abb_probe(struct platform_device *pdev)
 					       resource_size(res));
 	if (!abb->efuse_base) {
 		dev_err(dev, "Unable to map '%s'\n", pname);
-		ret = -ENOMEM;
-		goto err;
+		return -ENOMEM;
 	}
 
 	pname = "ldo-address";
@@ -776,10 +765,8 @@ static int ti_abb_probe(struct platform_device *pdev)
 		goto skip_opt;
 	}
 	abb->ldo_base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(abb->ldo_base)) {
-		ret = PTR_ERR(abb->ldo_base);
-		goto err;
-	}
+	if (IS_ERR(abb->ldo_base))
+		return PTR_ERR(abb->ldo_base);
 
 	/* IF ldo_base is set, the following are mandatory */
 	pname = "ti,ldovbb-override-mask";
@@ -788,12 +775,11 @@ static int ti_abb_probe(struct platform_device *pdev)
 				 &abb->ldovbb_override_mask);
 	if (ret) {
 		dev_err(dev, "Missing '%s' (%d)\n", pname, ret);
-		goto err;
+		return ret;
 	}
 	if (!abb->ldovbb_override_mask) {
 		dev_err(dev, "Invalid property:'%s' set as 0!\n", pname);
-		ret = -EINVAL;
-		goto err;
+		return -EINVAL;
 	}
 
 	pname = "ti,ldovbb-vset-mask";
@@ -802,12 +788,11 @@ static int ti_abb_probe(struct platform_device *pdev)
 				 &abb->ldovbb_vset_mask);
 	if (ret) {
 		dev_err(dev, "Missing '%s' (%d)\n", pname, ret);
-		goto err;
+		return ret;
 	}
 	if (!abb->ldovbb_vset_mask) {
 		dev_err(dev, "Invalid property:'%s' set as 0!\n", pname);
-		ret = -EINVAL;
-		goto err;
+		return -EINVAL;
 	}
 
 skip_opt:
@@ -817,31 +802,29 @@ skip_opt:
 				 &abb->txdone_mask);
 	if (ret) {
 		dev_err(dev, "Missing '%s' (%d)\n", pname, ret);
-		goto err;
+		return ret;
 	}
 	if (!abb->txdone_mask) {
 		dev_err(dev, "Invalid property:'%s' set as 0!\n", pname);
-		ret = -EINVAL;
-		goto err;
+		return -EINVAL;
 	}
 
 	initdata = of_get_regulator_init_data(dev, pdev->dev.of_node);
 	if (!initdata) {
-		ret = -ENOMEM;
 		dev_err(dev, "%s: Unable to alloc regulator init data\n",
 			__func__);
-		goto err;
+		return -ENOMEM;
 	}
 
 	/* init ABB opp_sel table */
 	ret = ti_abb_init_table(dev, abb, initdata);
 	if (ret)
-		goto err;
+		return ret;
 
 	/* init ABB timing */
 	ret = ti_abb_init_timings(dev, abb);
 	if (ret)
-		goto err;
+		return ret;
 
 	desc = &abb->rdesc;
 	desc->name = dev_name(dev);
@@ -859,12 +842,12 @@ skip_opt:
 	config.driver_data = abb;
 	config.of_node = pdev->dev.of_node;
 
-	rdev = regulator_register(desc, &config);
+	rdev = devm_regulator_register(dev, desc, &config);
 	if (IS_ERR(rdev)) {
 		ret = PTR_ERR(rdev);
 		dev_err(dev, "%s: failed to register regulator(%d)\n",
 			__func__, ret);
-		goto err;
+		return ret;
 	}
 	platform_set_drvdata(pdev, rdev);
 
@@ -872,31 +855,12 @@ skip_opt:
 	ti_abb_rmw(abb->regs->sr2_en_mask, 1, abb->regs->setup_reg, abb->base);
 
 	return 0;
-
-err:
-	dev_err(dev, "%s: Failed to initialize(%d)\n", __func__, ret);
-	return ret;
-}
-
-/**
- * ti_abb_remove() - cleanups
- * @pdev: ABB platform device
- *
- * Return: 0
- */
-static int ti_abb_remove(struct platform_device *pdev)
-{
-	struct regulator_dev *rdev = platform_get_drvdata(pdev);
-
-	regulator_unregister(rdev);
-	return 0;
 }
 
 MODULE_ALIAS("platform:ti_abb");
 
 static struct platform_driver ti_abb_driver = {
 	.probe = ti_abb_probe,
-	.remove = ti_abb_remove,
 	.driver = {
 		   .name = "ti_abb",
 		   .owner = THIS_MODULE,
-- 
1.7.9.5

Re: [PATCH 7/7] regulator: ti-abb: Use devm_regulator_register

[Nishanth Menon]

On 09/04/2013 01:31 AM, Sachin Kamat wrote:
> devm_* simplifies the code.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>

Seems to do a little more than devm_* alone, there is pruning of error
handling done here as well.

I am not comfortable with loosing print message information that help
debug issues on the field - which we usually have to based on just
kernel logs.

Please reduce the change to just using devm_regulator_register and
removal of .remove function and i am ok with that change.
> ---
>  drivers/regulator/ti-abb-regulator.c |   82 ++++++++++------------------------
>  1 file changed, 23 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/regulator/ti-abb-regulator.c b/drivers/regulator/ti-abb-regulator.c
> index 3753ed0..02392f8 100644
> --- a/drivers/regulator/ti-abb-regulator.c
> +++ b/drivers/regulator/ti-abb-regulator.c
> @@ -696,22 +696,17 @@ static int ti_abb_probe(struct platform_device *pdev)
>  	match = of_match_device(ti_abb_of_match, dev);
>  	if (!match) {
>  		/* We do not expect this to happen */
> -		ret = -ENODEV;
>  		dev_err(dev, "%s: Unable to match device\n", __func__);
> -		goto err;
> +		return -ENODEV;
>  	}
>  	if (!match->data) {
> -		ret = -EINVAL;
>  		dev_err(dev, "%s: Bad data in match\n", __func__);
> -		goto err;
> +		return -EINVAL;
>  	}
>  
>  	abb = devm_kzalloc(dev, sizeof(struct ti_abb), GFP_KERNEL);
> -	if (!abb) {
> -		dev_err(dev, "%s: Unable to allocate ABB struct\n", __func__);
> -		ret = -ENOMEM;
> -		goto err;
> -	}
> +	if (!abb)
> +		return -ENOMEM;
here.

>  	abb->regs = match->data;
>  
>  	/* Map ABB resources */
> @@ -719,21 +714,17 @@ static int ti_abb_probe(struct platform_device *pdev)
>  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
>  	if (!res) {
>  		dev_err(dev, "Missing '%s' IO resource\n", pname);
> -		ret = -ENODEV;
> -		goto err;
> +		return -ENODEV;
>  	}
>  	abb->base = devm_ioremap_resource(dev, res);
> -	if (IS_ERR(abb->base)) {
> -		ret = PTR_ERR(abb->base);
> -		goto err;
> -	}
> +	if (IS_ERR(abb->base))
> +		return PTR_ERR(abb->base);
>  
>  	pname = "int-address";
>  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
>  	if (!res) {
>  		dev_err(dev, "Missing '%s' IO resource\n", pname);
> -		ret = -ENODEV;
> -		goto err;
> +		return -ENODEV;
>  	}
>  	/*
>  	 * We may have shared interrupt register offsets which are
> @@ -743,8 +734,7 @@ static int ti_abb_probe(struct platform_device *pdev)
>  					     resource_size(res));
>  	if (!abb->int_base) {
>  		dev_err(dev, "Unable to map '%s'\n", pname);
> -		ret = -ENOMEM;
> -		goto err;
> +		return -ENOMEM;
>  	}
>  
>  	/* Map Optional resources */
> @@ -764,8 +754,7 @@ static int ti_abb_probe(struct platform_device *pdev)
>  					       resource_size(res));
>  	if (!abb->efuse_base) {
>  		dev_err(dev, "Unable to map '%s'\n", pname);
> -		ret = -ENOMEM;
> -		goto err;
> +		return -ENOMEM;
>  	}
>  
>  	pname = "ldo-address";
> @@ -776,10 +765,8 @@ static int ti_abb_probe(struct platform_device *pdev)
>  		goto skip_opt;
>  	}
>  	abb->ldo_base = devm_ioremap_resource(dev, res);
> -	if (IS_ERR(abb->ldo_base)) {
> -		ret = PTR_ERR(abb->ldo_base);
> -		goto err;
> -	}
> +	if (IS_ERR(abb->ldo_base))
> +		return PTR_ERR(abb->ldo_base);
>  
>  	/* IF ldo_base is set, the following are mandatory */
>  	pname = "ti,ldovbb-override-mask";
> @@ -788,12 +775,11 @@ static int ti_abb_probe(struct platform_device *pdev)
>  				 &abb->ldovbb_override_mask);
>  	if (ret) {
>  		dev_err(dev, "Missing '%s' (%d)\n", pname, ret);
> -		goto err;
> +		return ret;
>  	}
>  	if (!abb->ldovbb_override_mask) {
>  		dev_err(dev, "Invalid property:'%s' set as 0!\n", pname);
> -		ret = -EINVAL;
> -		goto err;
> +		return -EINVAL;
>  	}
>  
>  	pname = "ti,ldovbb-vset-mask";
> @@ -802,12 +788,11 @@ static int ti_abb_probe(struct platform_device *pdev)
>  				 &abb->ldovbb_vset_mask);
>  	if (ret) {
>  		dev_err(dev, "Missing '%s' (%d)\n", pname, ret);
> -		goto err;
> +		return ret;
>  	}
>  	if (!abb->ldovbb_vset_mask) {
>  		dev_err(dev, "Invalid property:'%s' set as 0!\n", pname);
> -		ret = -EINVAL;
> -		goto err;
> +		return -EINVAL;
>  	}
>  
>  skip_opt:
> @@ -817,31 +802,29 @@ skip_opt:
>  				 &abb->txdone_mask);
>  	if (ret) {
>  		dev_err(dev, "Missing '%s' (%d)\n", pname, ret);
> -		goto err;
> +		return ret;
>  	}
>  	if (!abb->txdone_mask) {
>  		dev_err(dev, "Invalid property:'%s' set as 0!\n", pname);
> -		ret = -EINVAL;
> -		goto err;
> +		return -EINVAL;
>  	}
>  
>  	initdata = of_get_regulator_init_data(dev, pdev->dev.of_node);
>  	if (!initdata) {
> -		ret = -ENOMEM;
>  		dev_err(dev, "%s: Unable to alloc regulator init data\n",
>  			__func__);
> -		goto err;
> +		return -ENOMEM;
>  	}
>  
>  	/* init ABB opp_sel table */
>  	ret = ti_abb_init_table(dev, abb, initdata);
>  	if (ret)
> -		goto err;
> +		return ret;
>  
>  	/* init ABB timing */
>  	ret = ti_abb_init_timings(dev, abb);
>  	if (ret)
> -		goto err;
> +		return ret;
>  
>  	desc = &abb->rdesc;
>  	desc->name = dev_name(dev);
> @@ -859,12 +842,12 @@ skip_opt:
>  	config.driver_data = abb;
>  	config.of_node = pdev->dev.of_node;
>  
> -	rdev = regulator_register(desc, &config);
> +	rdev = devm_regulator_register(dev, desc, &config);

ok with this.

>  	if (IS_ERR(rdev)) {
>  		ret = PTR_ERR(rdev);
>  		dev_err(dev, "%s: failed to register regulator(%d)\n",
>  			__func__, ret);
> -		goto err;
> +		return ret;
>  	}
>  	platform_set_drvdata(pdev, rdev);
>  
> @@ -872,31 +855,12 @@ skip_opt:
>  	ti_abb_rmw(abb->regs->sr2_en_mask, 1, abb->regs->setup_reg, abb->base);
>  
>  	return 0;
> -
> -err:
> -	dev_err(dev, "%s: Failed to initialize(%d)\n", __func__, ret);
> -	return ret;
here -> with this, the lazy non detailed prints end with a generic
fail case.

I can cleanup the error handling if you like.

> -}
> -
> -/**
> - * ti_abb_remove() - cleanups
> - * @pdev: ABB platform device
> - *
> - * Return: 0
> - */
> -static int ti_abb_remove(struct platform_device *pdev)
> -{
> -	struct regulator_dev *rdev = platform_get_drvdata(pdev);
> -
> -	regulator_unregister(rdev);
> -	return 0;
ok with this
>  }
>  
>  MODULE_ALIAS("platform:ti_abb");
>  
>  static struct platform_driver ti_abb_driver = {
>  	.probe = ti_abb_probe,
> -	.remove = ti_abb_remove,
ok with this
>  	.driver = {
>  		   .name = "ti_abb",
>  		   .owner = THIS_MODULE,
> 


-- 
Regards,
Nishanth Menon

Re: [PATCH 7/7] regulator: ti-abb: Use devm_regulator_register

[Mark Brown]

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

On Wed, Sep 04, 2013 at 08:27:56AM -0500, Nishanth Menon wrote:
> On 09/04/2013 01:31 AM, Sachin Kamat wrote:

> >  	abb = devm_kzalloc(dev, sizeof(struct ti_abb), GFP_KERNEL);
> > -	if (!abb) {
> > -		dev_err(dev, "%s: Unable to allocate ABB struct\n", __func__);
> > -		ret = -ENOMEM;
> > -		goto err;
> > -	}
> > +	if (!abb)
> > +		return -ENOMEM;

> here.

kzalloc() complains loudly when it fails, there's been a general thing
for removing the per-device logs as redundant.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 7/7] regulator: ti-abb: Use devm_regulator_register

[Nishanth Menon]

On 09/04/2013 10:12 AM, Mark Brown wrote:
> On Wed, Sep 04, 2013 at 08:27:56AM -0500, Nishanth Menon wrote:
>> On 09/04/2013 01:31 AM, Sachin Kamat wrote:
> 
>>>  	abb = devm_kzalloc(dev, sizeof(struct ti_abb), GFP_KERNEL);
>>> -	if (!abb) {
>>> -		dev_err(dev, "%s: Unable to allocate ABB struct\n", __func__);
>>> -		ret = -ENOMEM;
>>> -		goto err;
>>> -	}
>>> +	if (!abb)
>>> +		return -ENOMEM;
> 
>> here.
> 
> kzalloc() complains loudly when it fails, there's been a general thing
> for removing the per-device logs as redundant.
> 
True, but for the remaining, I do not thing it kind of helps to quiet
things up.

-- 
Regards,
Nishanth Menon

Re: [PATCH 7/7] regulator: ti-abb: Use devm_regulator_register

[Sachin Kamat]

Hi  Nishanth,

On 4 September 2013 18:57, Nishanth Menon <nm@ti.com> wrote:
> On 09/04/2013 01:31 AM, Sachin Kamat wrote:
>> -err:
>> -     dev_err(dev, "%s: Failed to initialize(%d)\n", __func__, ret);
>> -     return ret;
> here -> with this, the lazy non detailed prints end with a generic
> fail case.

Generally when a driver is converted to use devm_* APIs, the error
handling code gets
refactored and simplified as several unwinding calls get removed. The
above print was similarly removed as part
of the cleanup and refactoring as it did not add any extra value.
Probe failure is reported even without this error
message.

-- 
With warm regards,
Sachin

Re: [PATCH 7/7] regulator: ti-abb: Use devm_regulator_register

[Nishanth Menon]

On 09/04/2013 11:26 AM, Sachin Kamat wrote:
> Hi  Nishanth,
> 
> On 4 September 2013 18:57, Nishanth Menon <nm@ti.com> wrote:
>> On 09/04/2013 01:31 AM, Sachin Kamat wrote:
>>> -err:
>>> -     dev_err(dev, "%s: Failed to initialize(%d)\n", __func__, ret);
>>> -     return ret;
>> here -> with this, the lazy non detailed prints end with a generic
>> fail case.
> 
> Generally when a driver is converted to use devm_* APIs, the error
> handling code gets
> refactored and simplified as several unwinding calls get removed. The
> above print was similarly removed as part
> of the cleanup and refactoring as it did not add any extra value.
> Probe failure is reported even without this error
> message.
> 

the cleanups here do not have anything to do with devm optimization.
in fact, if we stick with what we have stated to do in this patch,
-	rdev = regulator_register(desc, &config);
+	rdev = devm_regulator_register(dev, desc, &config);
 	if (IS_ERR(rdev)) {
 		ret = PTR_ERR(rdev);
 		dev_err(dev, "%s: failed to register regulator(%d)\n",
 			__func__, ret);
-		goto err;
+		return ret;
 	}

is all we got to do in the probe path.

I accept that probe failure gives me the required info for fail, and
the cleanup of log prints also make sense, just that the log refactor
seems out of context to the specific change.

-- 
Regards,
Nishanth Menon

Re: [PATCH 7/7] regulator: ti-abb: Use devm_regulator_register

[Sachin Kamat]

On 4 September 2013 22:03, Nishanth Menon <nm@ti.com> wrote:
> On 09/04/2013 11:26 AM, Sachin Kamat wrote:

> I accept that probe failure gives me the required info for fail, and
> the cleanup of log prints also make sense, just that the log refactor
> seems out of context to the specific change.

As I mentioned earlier generally when you do these kind of
conversions, most of the trivial
clean-ups are also done along with it. Probably you would have
appreciated a mention about
this in the commit log too.

 --
With warm regards,
Sachin

Re: [PATCH 7/7] regulator: ti-abb: Use devm_regulator_register

[Nishanth Menon]

- yong.shen@linaro.org as it bounces.
On 09/04/2013 11:48 AM, Sachin Kamat wrote:
> On 4 September 2013 22:03, Nishanth Menon <nm@ti.com> wrote:
>> On 09/04/2013 11:26 AM, Sachin Kamat wrote:
> 
>> I accept that probe failure gives me the required info for fail, and
>> the cleanup of log prints also make sense, just that the log refactor
>> seems out of context to the specific change.
> 
> As I mentioned earlier generally when you do these kind of
> conversions, most of the trivial
> clean-ups are also done along with it. Probably you would have
> appreciated a mention about
> this in the commit log too.

yes, adding to commit log might have helped me. thanks for the cleanup
as well.


-- 
Regards,
Nishanth Menon

Re: [PATCH 7/7] regulator: ti-abb: Use devm_regulator_register

[Mark Brown]

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

On Wed, Sep 04, 2013 at 12:00:58PM -0500, Nishanth Menon wrote:

> > As I mentioned earlier generally when you do these kind of
> > conversions, most of the trivial
> > clean-ups are also done along with it. Probably you would have
> > appreciated a mention about
> > this in the commit log too.

> yes, adding to commit log might have helped me. thanks for the cleanup
> as well.

Indeed, that is good practice - either submit the changes separately or
mention all of them in the changelog.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/7] regulator: anatop-regulator: Use devm_regulator_register

[Mark Brown]

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

On Wed, Sep 04, 2013 at 12:00:57PM +0530, Sachin Kamat wrote:
> devm_* simplifies the code.

Applied all, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/7] regulator: anatop-regulator: Use devm_regulator_register

[Marek Vasut]

Dear Sachin Kamat,

> devm_* simplifies the code.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>

Makes sense

Acked-by: Marek Vasut <marex@denx.de>

> ---
> This series is compile tested.
> ---
>  drivers/regulator/anatop-regulator.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/regulator/anatop-regulator.c
> b/drivers/regulator/anatop-regulator.c index 0d4a8cc..e42bfd1 100644
> --- a/drivers/regulator/anatop-regulator.c
> +++ b/drivers/regulator/anatop-regulator.c
> @@ -200,7 +200,7 @@ static int anatop_regulator_probe(struct
> platform_device *pdev) config.regmap = sreg->anatop;
> 
>  	/* register regulator */
> -	rdev = regulator_register(rdesc, &config);
> +	rdev = devm_regulator_register(dev, rdesc, &config);
>  	if (IS_ERR(rdev)) {
>  		dev_err(dev, "failed to register %s\n",
>  			rdesc->name);
> @@ -223,7 +223,6 @@ static int anatop_regulator_remove(struct
> platform_device *pdev) struct anatop_regulator *sreg =
> rdev_get_drvdata(rdev);
>  	const char *name = sreg->name;
> 
> -	regulator_unregister(rdev);
>  	kfree(name);
> 
>  	return 0;

Best regards,
Marek Vasut