[PATCH 00/14] Assorted small patches for regulators

[PATCH 00/14] Assorted small patches for regulators

[Dmitry Torokhov]

Hi Liam,

I happend to peek into drivers/regulator and saw a bunch of small issues, so
here goes. The patches are against linux-next, compile-tested only since I
don't have the hardware,

Please consider applying.

Thanks,

Dmitry

---

Dmitry Torokhov (14):
      Regulators: max8925-regulator - clean up driver data after removal
      Regulators: wm8400 - cleanup platform driver data handling
      Regulators: wm8994 - clean up driver data after removal
      Regulators: wm831x-xxx - clean up driver data after removal
      Regulators: pcap-regulator - clean up driver data after removal
      Regulators: max8660 - annotate probe and remove methods
      Regulators: max1586 - annotate probe and remove methods
      Regulators: lp3971 - fail if platform data was not supplied
      Regulators: tps6507x-regulator - mark probe method as __devinit
      Regulators: tps65023-regulator - mark probe method as __devinit
      Regulators: twl-regulator - mark probe function as __devinit
      Regulators: fixed - annotate probe and remove methods
      Regulators: ab3100 - fix probe and remove annotations
      Regulators: virtual - use sysfs attribute groups


 drivers/regulator/ab3100.c             |    6 ++-
 drivers/regulator/fixed.c              |   19 +++++-----
 drivers/regulator/lp3971.c             |   58 +++++++++++++++--------------
 drivers/regulator/max1586.c            |    9 +++--
 drivers/regulator/max8660.c            |   11 +++---
 drivers/regulator/max8925-regulator.c  |    6 ++-
 drivers/regulator/pcap-regulator.c     |    8 +++-
 drivers/regulator/tps65023-regulator.c |   35 ++++++++----------
 drivers/regulator/tps6507x-regulator.c |   34 ++++++++---------
 drivers/regulator/twl-regulator.c      |    2 +
 drivers/regulator/virtual.c            |   64 +++++++++++++++++---------------
 drivers/regulator/wm831x-dcdc.c        |   12 ++++++
 drivers/regulator/wm831x-isink.c       |    3 ++
 drivers/regulator/wm831x-ldo.c         |    5 +++
 drivers/regulator/wm8400-regulator.c   |   13 ++++---
 drivers/regulator/wm8994-regulator.c   |   11 ++++--
 include/linux/mfd/wm8400.h             |    4 ++
 17 files changed, 164 insertions(+), 136 deletions(-)

[PATCH 01/14] Regulators: virtual - use sysfs attribute groups

[Dmitry Torokhov]

Instead of open-coding sysfs attribute group use canned solution.
Also add __devinit/__devexit markups for probe and remove methods
and use 'bool' where it makes sense.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/regulator/virtual.c |   64 ++++++++++++++++++++++---------------------
 1 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/drivers/regulator/virtual.c b/drivers/regulator/virtual.c
index addc032..d96ceca 100644
--- a/drivers/regulator/virtual.c
+++ b/drivers/regulator/virtual.c
@@ -19,7 +19,7 @@
 struct virtual_consumer_data {
 	struct mutex lock;
 	struct regulator *regulator;
-	int enabled;
+	bool enabled;
 	int min_uV;
 	int max_uV;
 	int min_uA;
@@ -49,7 +49,7 @@ static void update_voltage_constraints(struct device *dev,
 		dev_dbg(dev, "Enabling regulator\n");
 		ret = regulator_enable(data->regulator);
 		if (ret == 0)
-			data->enabled = 1;
+			data->enabled = true;
 		else
 			dev_err(dev, "regulator_enable() failed: %d\n",
 				ret);
@@ -59,7 +59,7 @@ static void update_voltage_constraints(struct device *dev,
 		dev_dbg(dev, "Disabling regulator\n");
 		ret = regulator_disable(data->regulator);
 		if (ret == 0)
-			data->enabled = 0;
+			data->enabled = false;
 		else
 			dev_err(dev, "regulator_disable() failed: %d\n",
 				ret);
@@ -89,7 +89,7 @@ static void update_current_limit_constraints(struct device *dev,
 		dev_dbg(dev, "Enabling regulator\n");
 		ret = regulator_enable(data->regulator);
 		if (ret == 0)
-			data->enabled = 1;
+			data->enabled = true;
 		else
 			dev_err(dev, "regulator_enable() failed: %d\n",
 				ret);
@@ -99,7 +99,7 @@ static void update_current_limit_constraints(struct device *dev,
 		dev_dbg(dev, "Disabling regulator\n");
 		ret = regulator_disable(data->regulator);
 		if (ret == 0)
-			data->enabled = 0;
+			data->enabled = false;
 		else
 			dev_err(dev, "regulator_disable() failed: %d\n",
 				ret);
@@ -270,24 +270,28 @@ static DEVICE_ATTR(min_microamps, 0666, show_min_uA, set_min_uA);
 static DEVICE_ATTR(max_microamps, 0666, show_max_uA, set_max_uA);
 static DEVICE_ATTR(mode, 0666, show_mode, set_mode);
 
-static struct device_attribute *attributes[] = {
-	&dev_attr_min_microvolts,
-	&dev_attr_max_microvolts,
-	&dev_attr_min_microamps,
-	&dev_attr_max_microamps,
-	&dev_attr_mode,
+static struct attribute *regulator_virtual_attributes[] = {
+	&dev_attr_min_microvolts.attr,
+	&dev_attr_max_microvolts.attr,
+	&dev_attr_min_microamps.attr,
+	&dev_attr_max_microamps.attr,
+	&dev_attr_mode.attr,
+	NULL
 };
 
-static int regulator_virtual_consumer_probe(struct platform_device *pdev)
+static const struct attribute_group regulator_virtual_attr_group = {
+	.attrs	= regulator_virtual_attributes,
+};
+
+static int __devinit regulator_virtual_probe(struct platform_device *pdev)
 {
 	char *reg_id = pdev->dev.platform_data;
 	struct virtual_consumer_data *drvdata;
-	int ret, i;
+	int ret;
 
 	drvdata = kzalloc(sizeof(struct virtual_consumer_data), GFP_KERNEL);
-	if (drvdata == NULL) {
+	if (drvdata == NULL)
 		return -ENOMEM;
-	}
 
 	mutex_init(&drvdata->lock);
 
@@ -299,13 +303,12 @@ static int regulator_virtual_consumer_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	for (i = 0; i < ARRAY_SIZE(attributes); i++) {
-		ret = device_create_file(&pdev->dev, attributes[i]);
-		if (ret != 0) {
-			dev_err(&pdev->dev, "Failed to create attr %d: %d\n",
-				i, ret);
-			goto err_regulator;
-		}
+	ret = sysfs_create_group(&pdev->dev.kobj,
+				 &regulator_virtual_attr_group);
+	if (ret != 0) {
+		dev_err(&pdev->dev,
+			"Failed to create attribute group: %d\n", ret);
+		goto err_regulator;
 	}
 
 	drvdata->mode = regulator_get_mode(drvdata->regulator);
@@ -317,37 +320,36 @@ static int regulator_virtual_consumer_probe(struct platform_device *pdev)
 err_regulator:
 	regulator_put(drvdata->regulator);
 err:
-	for (i = 0; i < ARRAY_SIZE(attributes); i++)
-		device_remove_file(&pdev->dev, attributes[i]);
 	kfree(drvdata);
 	return ret;
 }
 
-static int regulator_virtual_consumer_remove(struct platform_device *pdev)
+static int __devexit regulator_virtual_remove(struct platform_device *pdev)
 {
 	struct virtual_consumer_data *drvdata = platform_get_drvdata(pdev);
-	int i;
 
-	for (i = 0; i < ARRAY_SIZE(attributes); i++)
-		device_remove_file(&pdev->dev, attributes[i]);
+	sysfs_remove_group(&pdev->dev.kobj, &regulator_virtual_attr_group);
+
 	if (drvdata->enabled)
 		regulator_disable(drvdata->regulator);
 	regulator_put(drvdata->regulator);
 
 	kfree(drvdata);
 
+	platform_set_drvdata(pdev, NULL);
+
 	return 0;
 }
 
 static struct platform_driver regulator_virtual_consumer_driver = {
-	.probe		= regulator_virtual_consumer_probe,
-	.remove		= regulator_virtual_consumer_remove,
+	.probe		= regulator_virtual_probe,
+	.remove		= __devexit_p(regulator_virtual_remove),
 	.driver		= {
 		.name		= "reg-virt-consumer",
+		.owner		= THIS_MODULE,
 	},
 };
 
-
 static int __init regulator_virtual_consumer_init(void)
 {
 	return platform_driver_register(&regulator_virtual_consumer_driver);

Re: [PATCH 01/14] Regulators: virtual - use sysfs attribute groups

[Mark Brown]

On Tue, Feb 23, 2010 at 11:37:44PM -0800, Dmitry Torokhov wrote:
> Instead of open-coding sysfs attribute group use canned solution.
> Also add __devinit/__devexit markups for probe and remove methods
> and use 'bool' where it makes sense.

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH 01/14] Regulators: virtual - use sysfs attribute groups

[Liam Girdwood]

On Tue, 2010-02-23 at 23:37 -0800, Dmitry Torokhov wrote:
> Instead of open-coding sysfs attribute group use canned solution.
> Also add __devinit/__devexit markups for probe and remove methods
> and use 'bool' where it makes sense.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---

Applied.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH 02/14] Regulators: ab3100 - fix probe and remove annotations

[Dmitry Torokhov]

Probe and remove methods should not be marked as __init/__exit but
rather __devinit/__devexit so that the needed sections stay in memory
in presence of CONFIG_HOTPLUG. This is needed even on non hotpluggable
buses.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/regulator/ab3100.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/ab3100.c b/drivers/regulator/ab3100.c
index b349db4..7de9509 100644
--- a/drivers/regulator/ab3100.c
+++ b/drivers/regulator/ab3100.c
@@ -561,7 +561,7 @@ ab3100_regulator_desc[AB3100_NUM_REGULATORS] = {
  * for all the different regulators.
  */
 
-static int __init ab3100_regulators_probe(struct platform_device *pdev)
+static int __devinit ab3100_regulators_probe(struct platform_device *pdev)
 {
 	struct ab3100_platform_data *plfdata = pdev->dev.platform_data;
 	struct ab3100 *ab3100 = platform_get_drvdata(pdev);
@@ -641,7 +641,7 @@ static int __init ab3100_regulators_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int __exit ab3100_regulators_remove(struct platform_device *pdev)
+static int __devexit ab3100_regulators_remove(struct platform_device *pdev)
 {
 	int i;
 
@@ -659,7 +659,7 @@ static struct platform_driver ab3100_regulators_driver = {
 		.owner = THIS_MODULE,
 	},
 	.probe = ab3100_regulators_probe,
-	.remove = __exit_p(ab3100_regulators_remove),
+	.remove = __devexit_p(ab3100_regulators_remove),
 };
 
 static __init int ab3100_regulators_init(void)

Re: [PATCH 02/14] Regulators: ab3100 - fix probe and remove annotations

[Mark Brown]

On Tue, Feb 23, 2010 at 11:37:50PM -0800, Dmitry Torokhov wrote:
> Probe and remove methods should not be marked as __init/__exit but
> rather __devinit/__devexit so that the needed sections stay in memory
> in presence of CONFIG_HOTPLUG. This is needed even on non hotpluggable
> buses.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH 02/14] Regulators: ab3100 - fix probe and remove annotations

[Liam Girdwood]

On Tue, 2010-02-23 at 23:37 -0800, Dmitry Torokhov wrote:
> Probe and remove methods should not be marked as __init/__exit but
> rather __devinit/__devexit so that the needed sections stay in memory
> in presence of CONFIG_HOTPLUG. This is needed even on non hotpluggable
> buses.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Applied.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH 03/14] Regulators: fixed - annotate probe and remove methods

[Dmitry Torokhov]

Add __devinit/__devexit markings to probe and remove methids of the
driver, change types of variables containing boolean data to boolean,
set up driver's owner field so we have proper sysfs link between
driver and the module.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/regulator/fixed.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index a3d3bfc..d11f762 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -32,8 +32,8 @@ struct fixed_voltage_data {
 	int microvolts;
 	int gpio;
 	unsigned startup_delay;
-	unsigned enable_high:1;
-	unsigned is_enabled:1;
+	bool enable_high;
+	bool is_enabled;
 };
 
 static int fixed_voltage_is_enabled(struct regulator_dev *dev)
@@ -49,7 +49,7 @@ static int fixed_voltage_enable(struct regulator_dev *dev)
 
 	if (gpio_is_valid(data->gpio)) {
 		gpio_set_value_cansleep(data->gpio, data->enable_high);
-		data->is_enabled = 1;
+		data->is_enabled = true;
 	}
 
 	return 0;
@@ -61,7 +61,7 @@ static int fixed_voltage_disable(struct regulator_dev *dev)
 
 	if (gpio_is_valid(data->gpio)) {
 		gpio_set_value_cansleep(data->gpio, !data->enable_high);
-		data->is_enabled = 0;
+		data->is_enabled = false;
 	}
 
 	return 0;
@@ -101,7 +101,7 @@ static struct regulator_ops fixed_voltage_ops = {
 	.list_voltage = fixed_voltage_list_voltage,
 };
 
-static int regulator_fixed_voltage_probe(struct platform_device *pdev)
+static int __devinit reg_fixed_voltage_probe(struct platform_device *pdev)
 {
 	struct fixed_voltage_config *config = pdev->dev.platform_data;
 	struct fixed_voltage_data *drvdata;
@@ -174,7 +174,7 @@ static int regulator_fixed_voltage_probe(struct platform_device *pdev)
 		/* Regulator without GPIO control is considered
 		 * always enabled
 		 */
-		drvdata->is_enabled = 1;
+		drvdata->is_enabled = true;
 	}
 
 	drvdata->dev = regulator_register(&drvdata->desc, &pdev->dev,
@@ -202,7 +202,7 @@ err:
 	return ret;
 }
 
-static int regulator_fixed_voltage_remove(struct platform_device *pdev)
+static int __devexit reg_fixed_voltage_remove(struct platform_device *pdev)
 {
 	struct fixed_voltage_data *drvdata = platform_get_drvdata(pdev);
 
@@ -216,10 +216,11 @@ static int regulator_fixed_voltage_remove(struct platform_device *pdev)
 }
 
 static struct platform_driver regulator_fixed_voltage_driver = {
-	.probe		= regulator_fixed_voltage_probe,
-	.remove		= regulator_fixed_voltage_remove,
+	.probe		= reg_fixed_voltage_probe,
+	.remove		= __devexit_p(reg_fixed_voltage_remove),
 	.driver		= {
 		.name		= "reg-fixed-voltage",
+		.owner		= THIS_MODULE,
 	},
 };
 

Re: [PATCH 03/14] Regulators: fixed - annotate probe and remove methods

[Mark Brown]

On Tue, Feb 23, 2010 at 11:37:55PM -0800, Dmitry Torokhov wrote:
> Add __devinit/__devexit markings to probe and remove methids of the
> driver, change types of variables containing boolean data to boolean,
> set up driver's owner field so we have proper sysfs link between
> driver and the module.

> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH 03/14] Regulators: fixed - annotate probe and remove methods

[Liam Girdwood]

On Tue, 2010-02-23 at 23:37 -0800, Dmitry Torokhov wrote:
> Add __devinit/__devexit markings to probe and remove methids of the
> driver, change types of variables containing boolean data to boolean,
> set up driver's owner field so we have proper sysfs link between
> driver and the module.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Applied.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH 04/14] Regulators: twl-regulator - mark probe function as __devinit

[Dmitry Torokhov]

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/regulator/twl-regulator.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
index 5f394bb..9729d76 100644
--- a/drivers/regulator/twl-regulator.c
+++ b/drivers/regulator/twl-regulator.c
@@ -531,7 +531,7 @@ static struct twlreg_info twl_regs[] = {
 	TWL6030_FIXED_LDO(VUSB, 0x70, 3300, 18, 0, 0x21)
 };
 
-static int twlreg_probe(struct platform_device *pdev)
+static int __devinit twlreg_probe(struct platform_device *pdev)
 {
 	int				i;
 	struct twlreg_info		*info;

Re: [PATCH 04/14] Regulators: twl-regulator - mark probe function as __devinit

[Mark Brown]

On Tue, Feb 23, 2010 at 11:38:01PM -0800, Dmitry Torokhov wrote:
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH 04/14] Regulators: twl-regulator - mark probe function as __devinit

[Liam Girdwood]

On Tue, 2010-02-23 at 23:38 -0800, Dmitry Torokhov wrote:
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
> 
>  drivers/regulator/twl-regulator.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
> index 5f394bb..9729d76 100644
> --- a/drivers/regulator/twl-regulator.c
> +++ b/drivers/regulator/twl-regulator.c
> @@ -531,7 +531,7 @@ static struct twlreg_info twl_regs[] = {
>  	TWL6030_FIXED_LDO(VUSB, 0x70, 3300, 18, 0, 0x21)
>  };
>  
> -static int twlreg_probe(struct platform_device *pdev)
> +static int __devinit twlreg_probe(struct platform_device *pdev)
>  {
>  	int				i;
>  	struct twlreg_info		*info;
> 

Applied.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH 05/14] Regulators: tps65023-regulator - mark probe method as __devinit

[Dmitry Torokhov]

Also move error handling in probe() out of line and do not bother
to reset fields in structures that are about to be freed.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/regulator/tps65023-regulator.c |   35 +++++++++++++++-----------------
 1 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/regulator/tps65023-regulator.c b/drivers/regulator/tps65023-regulator.c
index 07fda0a..1f18354 100644
--- a/drivers/regulator/tps65023-regulator.c
+++ b/drivers/regulator/tps65023-regulator.c
@@ -457,8 +457,8 @@ static struct regulator_ops tps65023_ldo_ops = {
 	.list_voltage = tps65023_ldo_list_voltage,
 };
 
-static
-int tps_65023_probe(struct i2c_client *client, const struct i2c_device_id *id)
+static int __devinit tps_65023_probe(struct i2c_client *client,
+				     const struct i2c_device_id *id)
 {
 	static int desc_id;
 	const struct tps_info *info = (void *)id->driver_data;
@@ -466,6 +466,7 @@ int tps_65023_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	struct regulator_dev *rdev;
 	struct tps_pmic *tps;
 	int i;
+	int error;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -EIO;
@@ -475,7 +476,6 @@ int tps_65023_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	 * coming from the board-evm file.
 	 */
 	init_data = client->dev.platform_data;
-
 	if (!init_data)
 		return -EIO;
 
@@ -502,21 +502,12 @@ int tps_65023_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 		/* Register the regulators */
 		rdev = regulator_register(&tps->desc[i], &client->dev,
-								init_data, tps);
+					  init_data, tps);
 		if (IS_ERR(rdev)) {
 			dev_err(&client->dev, "failed to register %s\n",
 				id->name);
-
-			/* Unregister */
-			while (i)
-				regulator_unregister(tps->rdev[--i]);
-
-			tps->client = NULL;
-
-			/* clear the client data in i2c */
-			i2c_set_clientdata(client, NULL);
-			kfree(tps);
-			return PTR_ERR(rdev);
+			error = PTR_ERR(rdev);
+			goto fail;
 		}
 
 		/* Save regulator for cleanup */
@@ -526,6 +517,13 @@ int tps_65023_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	i2c_set_clientdata(client, tps);
 
 	return 0;
+
+ fail:
+	while (--i >= 0)
+		regulator_unregister(tps->rdev[i]);
+
+	kfree(tps);
+	return error;
 }
 
 /**
@@ -539,13 +537,12 @@ static int __devexit tps_65023_remove(struct i2c_client *client)
 	struct tps_pmic *tps = i2c_get_clientdata(client);
 	int i;
 
+	/* clear the client data in i2c */
+	i2c_set_clientdata(client, NULL);
+
 	for (i = 0; i < TPS65023_NUM_REGULATOR; i++)
 		regulator_unregister(tps->rdev[i]);
 
-	tps->client = NULL;
-
-	/* clear the client data in i2c */
-	i2c_set_clientdata(client, NULL);
 	kfree(tps);
 
 	return 0;

Re: [PATCH 05/14] Regulators: tps65023-regulator - mark probe method as __devinit

[Mark Brown]

On Tue, Feb 23, 2010 at 11:38:06PM -0800, Dmitry Torokhov wrote:
> Also move error handling in probe() out of line and do not bother
> to reset fields in structures that are about to be freed.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH 05/14] Regulators: tps65023-regulator - mark probe method as __devinit

[Liam Girdwood]

On Tue, 2010-02-23 at 23:38 -0800, Dmitry Torokhov wrote:
> Also move error handling in probe() out of line and do not bother
> to reset fields in structures that are about to be freed.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---

Applied.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH 06/14] Regulators: tps6507x-regulator - mark probe method as __devinit

[Dmitry Torokhov]

Also move error handling in probe() out of line and do not bother
to reset fields in structures that are about to be freed.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/regulator/tps6507x-regulator.c |   34 ++++++++++++++------------------
 1 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/regulator/tps6507x-regulator.c b/drivers/regulator/tps6507x-regulator.c
index f8a6dfb..c2a9539 100644
--- a/drivers/regulator/tps6507x-regulator.c
+++ b/drivers/regulator/tps6507x-regulator.c
@@ -538,8 +538,8 @@ static struct regulator_ops tps6507x_ldo_ops = {
 	.list_voltage = tps6507x_ldo_list_voltage,
 };
 
-static
-int tps_6507x_probe(struct i2c_client *client, const struct i2c_device_id *id)
+static int __devinit tps_6507x_probe(struct i2c_client *client,
+				     const struct i2c_device_id *id)
 {
 	static int desc_id;
 	const struct tps_info *info = (void *)id->driver_data;
@@ -547,6 +547,7 @@ int tps_6507x_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	struct regulator_dev *rdev;
 	struct tps_pmic *tps;
 	int i;
+	int error;
 
 	if (!i2c_check_functionality(client->adapter,
 				I2C_FUNC_SMBUS_BYTE_DATA))
@@ -557,7 +558,6 @@ int tps_6507x_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	 * coming from the board-evm file.
 	 */
 	init_data = client->dev.platform_data;
-
 	if (!init_data)
 		return -EIO;
 
@@ -586,18 +586,8 @@ int tps_6507x_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		if (IS_ERR(rdev)) {
 			dev_err(&client->dev, "failed to register %s\n",
 				id->name);
-
-			/* Unregister */
-			while (i)
-				regulator_unregister(tps->rdev[--i]);
-
-			tps->client = NULL;
-
-			/* clear the client data in i2c */
-			i2c_set_clientdata(client, NULL);
-
-			kfree(tps);
-			return PTR_ERR(rdev);
+			error = PTR_ERR(rdev);
+			goto fail;
 		}
 
 		/* Save regulator for cleanup */
@@ -607,6 +597,13 @@ int tps_6507x_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	i2c_set_clientdata(client, tps);
 
 	return 0;
+
+fail:
+	while (--i >= 0)
+		regulator_unregister(tps->rdev[i]);
+
+	kfree(tps);
+	return error;
 }
 
 /**
@@ -620,13 +617,12 @@ static int __devexit tps_6507x_remove(struct i2c_client *client)
 	struct tps_pmic *tps = i2c_get_clientdata(client);
 	int i;
 
+	/* clear the client data in i2c */
+	i2c_set_clientdata(client, NULL);
+
 	for (i = 0; i < TPS6507X_NUM_REGULATOR; i++)
 		regulator_unregister(tps->rdev[i]);
 
-	tps->client = NULL;
-
-	/* clear the client data in i2c */
-	i2c_set_clientdata(client, NULL);
 	kfree(tps);
 
 	return 0;

Re: [PATCH 06/14] Regulators: tps6507x-regulator - mark probe method as __devinit

[Mark Brown]

On Tue, Feb 23, 2010 at 11:38:12PM -0800, Dmitry Torokhov wrote:
> Also move error handling in probe() out of line and do not bother
> to reset fields in structures that are about to be freed.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH 06/14] Regulators: tps6507x-regulator - mark probe method as __devinit

[Liam Girdwood]

On Tue, 2010-02-23 at 23:38 -0800, Dmitry Torokhov wrote:
> Also move error handling in probe() out of line and do not bother
> to reset fields in structures that are about to be freed.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
> 

Applied.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH 07/14] Regulators: lp3971 - fail if platform data was not supplied

[Dmitry Torokhov]

There is no point in completing probe if platform data is missing so
let's abort loading early.

Also, use kcalloc when allocating several instances of the same data
structure and mark setup_regulators() as __devinit since it is only
called from lp3971_i2c_probe() which is __devinit.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/regulator/lp3971.c |   58 ++++++++++++++++++++++----------------------
 1 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/regulator/lp3971.c b/drivers/regulator/lp3971.c
index 3ea639f..f5532ed 100644
--- a/drivers/regulator/lp3971.c
+++ b/drivers/regulator/lp3971.c
@@ -431,20 +431,20 @@ static int lp3971_set_bits(struct lp3971 *lp3971, u8 reg, u16 mask, u16 val)
 	return ret;
 }
 
-static int setup_regulators(struct lp3971 *lp3971,
-	struct lp3971_platform_data *pdata)
+static int __devinit setup_regulators(struct lp3971 *lp3971,
+				      struct lp3971_platform_data *pdata)
 {
 	int i, err;
-	int num_regulators = pdata->num_regulators;
-	lp3971->num_regulators = num_regulators;
-	lp3971->rdev = kzalloc(sizeof(struct regulator_dev *) * num_regulators,
-		GFP_KERNEL);
+
+	lp3971->num_regulators = pdata->num_regulators;
+	lp3971->rdev = kcalloc(pdata->num_regulators,
+				sizeof(struct regulator_dev *), GFP_KERNEL);
 
 	/* Instantiate the regulators */
-	for (i = 0; i < num_regulators; i++) {
-		int id = pdata->regulators[i].id;
-		lp3971->rdev[i] = regulator_register(&regulators[id],
-			lp3971->dev, pdata->regulators[i].initdata, lp3971);
+	for (i = 0; i < pdata->num_regulators; i++) {
+		struct lp3971_regulator_subdev *reg = &pdata->regulators[i];
+		lp3971->rdev[i] = regulator_register(&regulators[reg->id],
+					lp3971->dev, reg->initdata, lp3971);
 
 		if (IS_ERR(lp3971->rdev[i])) {
 			err = PTR_ERR(lp3971->rdev[i]);
@@ -455,10 +455,10 @@ static int setup_regulators(struct lp3971 *lp3971,
 	}
 
 	return 0;
+
 error:
-	for (i = 0; i < num_regulators; i++)
-		if (lp3971->rdev[i])
-			regulator_unregister(lp3971->rdev[i]);
+	while (--i >= 0)
+		regulator_unregister(lp3971->rdev[i]);
 	kfree(lp3971->rdev);
 	lp3971->rdev = NULL;
 	return err;
@@ -472,15 +472,17 @@ static int __devinit lp3971_i2c_probe(struct i2c_client *i2c,
 	int ret;
 	u16 val;
 
-	lp3971 = kzalloc(sizeof(struct lp3971), GFP_KERNEL);
-	if (lp3971 == NULL) {
-		ret = -ENOMEM;
-		goto err;
+	if (!pdata) {
+		dev_dbg(&i2c->dev, "No platform init data supplied\n");
+		return -ENODEV;
 	}
 
+	lp3971 = kzalloc(sizeof(struct lp3971), GFP_KERNEL);
+	if (lp3971 == NULL)
+		return -ENOMEM;
+
 	lp3971->i2c = i2c;
 	lp3971->dev = &i2c->dev;
-	i2c_set_clientdata(i2c, lp3971);
 
 	mutex_init(&lp3971->io_lock);
 
@@ -493,19 +495,15 @@ static int __devinit lp3971_i2c_probe(struct i2c_client *i2c,
 		goto err_detect;
 	}
 
-	if (pdata) {
-		ret = setup_regulators(lp3971, pdata);
-		if (ret < 0)
-			goto err_detect;
-	} else
-		dev_warn(lp3971->dev, "No platform init data supplied\n");
+	ret = setup_regulators(lp3971, pdata);
+	if (ret < 0)
+		goto err_detect;
 
+	i2c_set_clientdata(i2c, lp3971);
 	return 0;
 
 err_detect:
-	i2c_set_clientdata(i2c, NULL);
 	kfree(lp3971);
-err:
 	return ret;
 }
 
@@ -513,11 +511,13 @@ static int __devexit lp3971_i2c_remove(struct i2c_client *i2c)
 {
 	struct lp3971 *lp3971 = i2c_get_clientdata(i2c);
 	int i;
+
+	i2c_set_clientdata(i2c, NULL);
+
 	for (i = 0; i < lp3971->num_regulators; i++)
-		if (lp3971->rdev[i])
-			regulator_unregister(lp3971->rdev[i]);
+		regulator_unregister(lp3971->rdev[i]);
+
 	kfree(lp3971->rdev);
-	i2c_set_clientdata(i2c, NULL);
 	kfree(lp3971);
 
 	return 0;

Re: [PATCH 07/14] Regulators: lp3971 - fail if platform data was not supplied

[Mark Brown]

On Tue, Feb 23, 2010 at 11:38:17PM -0800, Dmitry Torokhov wrote:
> There is no point in completing probe if platform data is missing so
> let's abort loading early.
> 
> Also, use kcalloc when allocating several instances of the same data
> structure and mark setup_regulators() as __devinit since it is only
> called from lp3971_i2c_probe() which is __devinit.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

but this *really* should have been split into multiple patches, there's
a large number of different changes mixed in here, with random coding
style tweaks overlapping with substantial changes to the handling of
probe.

Re: [PATCH 07/14] Regulators: lp3971 - fail if platform data was not supplied

[Liam Girdwood]

On Tue, 2010-02-23 at 23:38 -0800, Dmitry Torokhov wrote:
> There is no point in completing probe if platform data is missing so
> let's abort loading early.
> 
> Also, use kcalloc when allocating several instances of the same data
> structure and mark setup_regulators() as __devinit since it is only
> called from lp3971_i2c_probe() which is __devinit.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Applied.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH 08/14] Regulators: max1586 - annotate probe and remove methods

[Dmitry Torokhov]

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/regulator/max1586.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/max1586.c b/drivers/regulator/max1586.c
index 2c082d3..a49fc95 100644
--- a/drivers/regulator/max1586.c
+++ b/drivers/regulator/max1586.c
@@ -179,8 +179,8 @@ static struct regulator_desc max1586_reg[] = {
 	},
 };
 
-static int max1586_pmic_probe(struct i2c_client *client,
-			      const struct i2c_device_id *i2c_id)
+static int __devinit max1586_pmic_probe(struct i2c_client *client,
+					const struct i2c_device_id *i2c_id)
 {
 	struct regulator_dev **rdev;
 	struct max1586_platform_data *pdata = client->dev.platform_data;
@@ -235,7 +235,7 @@ out:
 	return ret;
 }
 
-static int max1586_pmic_remove(struct i2c_client *client)
+static int __devexit max1586_pmic_remove(struct i2c_client *client)
 {
 	struct regulator_dev **rdev = i2c_get_clientdata(client);
 	int i;
@@ -257,9 +257,10 @@ MODULE_DEVICE_TABLE(i2c, max1586_id);
 
 static struct i2c_driver max1586_pmic_driver = {
 	.probe = max1586_pmic_probe,
-	.remove = max1586_pmic_remove,
+	.remove = __devexit_p(max1586_pmic_remove),
 	.driver		= {
 		.name	= "max1586",
+		.owner	= THIS_MODULE,
 	},
 	.id_table	= max1586_id,
 };

Re: [PATCH 08/14] Regulators: max1586 - annotate probe and remove methods

[Mark Brown]

On Tue, Feb 23, 2010 at 11:38:23PM -0800, Dmitry Torokhov wrote:
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

>  	.driver		= {
>  		.name	= "max1586",
> +		.owner	= THIS_MODULE,
>  	},

Note that a large number of your patches introduce this change and none
of them note it in the changelog...

Re: [PATCH 08/14] Regulators: max1586 - annotate probe and remove methods

[Dmitry Torokhov]

Hi Mark,

On Wednesday 24 February 2010 02:06:55 am Mark Brown wrote:
> On Tue, Feb 23, 2010 at 11:38:23PM -0800, Dmitry Torokhov wrote:
> > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> 
> Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> 
> >  	.driver		= {
> >  		.name	= "max1586",
> > +		.owner	= THIS_MODULE,
> >  	},
> 
> Note that a large number of your patches introduce this change and none
> of them note it in the changelog...
> 

Thanks for reviewing the lot.

Yes, I quite often list only most important changes in the log, and just
throw stuff like this in because I prefer to work on driver by driver
basis and splitting everything up produce twice as many patches. I will
try to be more detailed in my future submissions. 

-- 
Dmitry

Re: [PATCH 08/14] Regulators: max1586 - annotate probe and remove methods

[Liam Girdwood]

On Tue, 2010-02-23 at 23:38 -0800, Dmitry Torokhov wrote:
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
> 
>  drivers/regulator/max1586.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/regulator/max1586.c b/drivers/regulator/max1586.c
> index 2c082d3..a49fc95 100644
> --- a/drivers/regulator/max1586.c
> +++ b/drivers/regulator/max1586.c
> @@ -179,8 +179,8 @@ static struct regulator_desc max1586_reg[] = {
>  	},
>  };
>  
> -static int max1586_pmic_probe(struct i2c_client *client,
> -			      const struct i2c_device_id *i2c_id)
> +static int __devinit max1586_pmic_probe(struct i2c_client *client,
> +					const struct i2c_device_id *i2c_id)
>  {
>  	struct regulator_dev **rdev;
>  	struct max1586_platform_data *pdata = client->dev.platform_data;
> @@ -235,7 +235,7 @@ out:
>  	return ret;
>  }
>  
> -static int max1586_pmic_remove(struct i2c_client *client)
> +static int __devexit max1586_pmic_remove(struct i2c_client *client)
>  {
>  	struct regulator_dev **rdev = i2c_get_clientdata(client);
>  	int i;
> @@ -257,9 +257,10 @@ MODULE_DEVICE_TABLE(i2c, max1586_id);
>  
>  static struct i2c_driver max1586_pmic_driver = {
>  	.probe = max1586_pmic_probe,
> -	.remove = max1586_pmic_remove,
> +	.remove = __devexit_p(max1586_pmic_remove),
>  	.driver		= {
>  		.name	= "max1586",
> +		.owner	= THIS_MODULE,
>  	},
>  	.id_table	= max1586_id,
>  };
> 

Applied.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH 09/14] Regulators: max8660 - annotate probe and remove methods

[Dmitry Torokhov]

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/regulator/max8660.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/max8660.c b/drivers/regulator/max8660.c
index acc2fb7..f12f1bb 100644
--- a/drivers/regulator/max8660.c
+++ b/drivers/regulator/max8660.c
@@ -345,8 +345,8 @@ static struct regulator_desc max8660_reg[] = {
 	},
 };
 
-static int max8660_probe(struct i2c_client *client,
-			      const struct i2c_device_id *i2c_id)
+static int __devinit max8660_probe(struct i2c_client *client,
+				   const struct i2c_device_id *i2c_id)
 {
 	struct regulator_dev **rdev;
 	struct max8660_platform_data *pdata = client->dev.platform_data;
@@ -354,7 +354,7 @@ static int max8660_probe(struct i2c_client *client,
 	int boot_on, i, id, ret = -EINVAL;
 
 	if (pdata->num_subdevs > MAX8660_V_END) {
-		dev_err(&client->dev, "Too much regulators found!\n");
+		dev_err(&client->dev, "Too many regulators found!\n");
 		goto out;
 	}
 
@@ -462,7 +462,7 @@ out:
 	return ret;
 }
 
-static int max8660_remove(struct i2c_client *client)
+static int __devexit max8660_remove(struct i2c_client *client)
 {
 	struct regulator_dev **rdev = i2c_get_clientdata(client);
 	int i;
@@ -485,9 +485,10 @@ MODULE_DEVICE_TABLE(i2c, max8660_id);
 
 static struct i2c_driver max8660_driver = {
 	.probe = max8660_probe,
-	.remove = max8660_remove,
+	.remove = __devexit_p(max8660_remove),
 	.driver		= {
 		.name	= "max8660",
+		.owner	= THIS_MODULE,
 	},
 	.id_table	= max8660_id,
 };

Re: [PATCH 09/14] Regulators: max8660 - annotate probe and remove methods

[Mark Brown]

On Tue, Feb 23, 2010 at 11:38:28PM -0800, Dmitry Torokhov wrote:
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH 09/14] Regulators: max8660 - annotate probe and remove methods

[Liam Girdwood]

On Tue, 2010-02-23 at 23:38 -0800, Dmitry Torokhov wrote:
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
> 
>  drivers/regulator/max8660.c |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/regulator/max8660.c b/drivers/regulator/max8660.c
> index acc2fb7..f12f1bb 100644
> --- a/drivers/regulator/max8660.c
> +++ b/drivers/regulator/max8660.c
> @@ -345,8 +345,8 @@ static struct regulator_desc max8660_reg[] = {
>  	},
>  };
>  
> -static int max8660_probe(struct i2c_client *client,
> -			      const struct i2c_device_id *i2c_id)
> +static int __devinit max8660_probe(struct i2c_client *client,
> +				   const struct i2c_device_id *i2c_id)
>  {
>  	struct regulator_dev **rdev;
>  	struct max8660_platform_data *pdata = client->dev.platform_data;
> @@ -354,7 +354,7 @@ static int max8660_probe(struct i2c_client *client,
>  	int boot_on, i, id, ret = -EINVAL;
>  
>  	if (pdata->num_subdevs > MAX8660_V_END) {
> -		dev_err(&client->dev, "Too much regulators found!\n");
> +		dev_err(&client->dev, "Too many regulators found!\n");
>  		goto out;
>  	}
>  
> @@ -462,7 +462,7 @@ out:
>  	return ret;
>  }
>  
> -static int max8660_remove(struct i2c_client *client)
> +static int __devexit max8660_remove(struct i2c_client *client)
>  {
>  	struct regulator_dev **rdev = i2c_get_clientdata(client);
>  	int i;
> @@ -485,9 +485,10 @@ MODULE_DEVICE_TABLE(i2c, max8660_id);
>  
>  static struct i2c_driver max8660_driver = {
>  	.probe = max8660_probe,
> -	.remove = max8660_remove,
> +	.remove = __devexit_p(max8660_remove),
>  	.driver		= {
>  		.name	= "max8660",
> +		.owner	= THIS_MODULE,
>  	},
>  	.id_table	= max8660_id,
>  };
> 

Applied.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH 10/14] Regulators: pcap-regulator - clean up driver data after removal

[Dmitry Torokhov]

It is a good tone to reset driver data after unbinding the device.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/regulator/pcap-regulator.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/pcap-regulator.c b/drivers/regulator/pcap-regulator.c
index 33d7d89..29d0566 100644
--- a/drivers/regulator/pcap-regulator.c
+++ b/drivers/regulator/pcap-regulator.c
@@ -288,16 +288,18 @@ static int __devexit pcap_regulator_remove(struct platform_device *pdev)
 	struct regulator_dev *rdev = platform_get_drvdata(pdev);
 
 	regulator_unregister(rdev);
+	platform_set_drvdata(pdev, NULL);
 
 	return 0;
 }
 
 static struct platform_driver pcap_regulator_driver = {
 	.driver = {
-		.name = "pcap-regulator",
+		.name	= "pcap-regulator",
+		.owner	= THIS_MODULE,
 	},
-	.probe = pcap_regulator_probe,
-	.remove = __devexit_p(pcap_regulator_remove),
+	.probe	= pcap_regulator_probe,
+	.remove	= __devexit_p(pcap_regulator_remove),
 };
 
 static int __init pcap_regulator_init(void)

Re: [PATCH 10/14] Regulators: pcap-regulator - clean up driver data after removal

[Mark Brown]

On Tue, Feb 23, 2010 at 11:38:33PM -0800, Dmitry Torokhov wrote:
> It is a good tone to reset driver data after unbinding the device.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH 10/14] Regulators: pcap-regulator - clean up driver data after removal

[Liam Girdwood]

On Tue, 2010-02-23 at 23:38 -0800, Dmitry Torokhov wrote:
> It is a good tone to reset driver data after unbinding the device.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
> 
>  drivers/regulator/pcap-regulator.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
> 

Applied.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH 11/14] Regulators: wm831x-xxx - clean up driver data after removal

[Dmitry Torokhov]

It is a good tone to reset driver data after unbinding the device.
Also set up drivers owner.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/regulator/wm831x-dcdc.c  |   12 ++++++++++++
 drivers/regulator/wm831x-isink.c |    3 +++
 drivers/regulator/wm831x-ldo.c   |    5 +++++
 3 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/wm831x-dcdc.c b/drivers/regulator/wm831x-dcdc.c
index 0a65775..6e18e56 100644
--- a/drivers/regulator/wm831x-dcdc.c
+++ b/drivers/regulator/wm831x-dcdc.c
@@ -600,6 +600,8 @@ static __devexit int wm831x_buckv_remove(struct platform_device *pdev)
 	struct wm831x_dcdc *dcdc = platform_get_drvdata(pdev);
 	struct wm831x *wm831x = dcdc->wm831x;
 
+	platform_set_drvdata(pdev, NULL);
+
 	wm831x_free_irq(wm831x, platform_get_irq_byname(pdev, "HC"), dcdc);
 	wm831x_free_irq(wm831x, platform_get_irq_byname(pdev, "UV"), dcdc);
 	regulator_unregister(dcdc->regulator);
@@ -615,6 +617,7 @@ static struct platform_driver wm831x_buckv_driver = {
 	.remove = __devexit_p(wm831x_buckv_remove),
 	.driver		= {
 		.name	= "wm831x-buckv",
+		.owner	= THIS_MODULE,
 	},
 };
 
@@ -769,6 +772,8 @@ static __devexit int wm831x_buckp_remove(struct platform_device *pdev)
 	struct wm831x_dcdc *dcdc = platform_get_drvdata(pdev);
 	struct wm831x *wm831x = dcdc->wm831x;
 
+	platform_set_drvdata(pdev, NULL);
+
 	wm831x_free_irq(wm831x, platform_get_irq_byname(pdev, "UV"), dcdc);
 	regulator_unregister(dcdc->regulator);
 	kfree(dcdc);
@@ -781,6 +786,7 @@ static struct platform_driver wm831x_buckp_driver = {
 	.remove = __devexit_p(wm831x_buckp_remove),
 	.driver		= {
 		.name	= "wm831x-buckp",
+		.owner	= THIS_MODULE,
 	},
 };
 
@@ -895,6 +901,8 @@ static __devexit int wm831x_boostp_remove(struct platform_device *pdev)
 	struct wm831x_dcdc *dcdc = platform_get_drvdata(pdev);
 	struct wm831x *wm831x = dcdc->wm831x;
 
+	platform_set_drvdata(pdev, NULL);
+
 	wm831x_free_irq(wm831x, platform_get_irq_byname(pdev, "UV"), dcdc);
 	regulator_unregister(dcdc->regulator);
 	kfree(dcdc);
@@ -907,6 +915,7 @@ static struct platform_driver wm831x_boostp_driver = {
 	.remove = __devexit_p(wm831x_boostp_remove),
 	.driver		= {
 		.name	= "wm831x-boostp",
+		.owner	= THIS_MODULE,
 	},
 };
 
@@ -979,6 +988,8 @@ static __devexit int wm831x_epe_remove(struct platform_device *pdev)
 {
 	struct wm831x_dcdc *dcdc = platform_get_drvdata(pdev);
 
+	platform_set_drvdata(pdev, NULL);
+
 	regulator_unregister(dcdc->regulator);
 	kfree(dcdc);
 
@@ -990,6 +1001,7 @@ static struct platform_driver wm831x_epe_driver = {
 	.remove = __devexit_p(wm831x_epe_remove),
 	.driver		= {
 		.name	= "wm831x-epe",
+		.owner	= THIS_MODULE,
 	},
 };
 
diff --git a/drivers/regulator/wm831x-isink.c b/drivers/regulator/wm831x-isink.c
index 4885700..ca0f6b6 100644
--- a/drivers/regulator/wm831x-isink.c
+++ b/drivers/regulator/wm831x-isink.c
@@ -222,6 +222,8 @@ static __devexit int wm831x_isink_remove(struct platform_device *pdev)
 	struct wm831x_isink *isink = platform_get_drvdata(pdev);
 	struct wm831x *wm831x = isink->wm831x;
 
+	platform_set_drvdata(pdev, NULL);
+
 	wm831x_free_irq(wm831x, platform_get_irq(pdev, 0), isink);
 
 	regulator_unregister(isink->regulator);
@@ -235,6 +237,7 @@ static struct platform_driver wm831x_isink_driver = {
 	.remove = __devexit_p(wm831x_isink_remove),
 	.driver		= {
 		.name	= "wm831x-isink",
+		.owner	= THIS_MODULE,
 	},
 };
 
diff --git a/drivers/regulator/wm831x-ldo.c b/drivers/regulator/wm831x-ldo.c
index 61e02ac..d2406c1 100644
--- a/drivers/regulator/wm831x-ldo.c
+++ b/drivers/regulator/wm831x-ldo.c
@@ -371,6 +371,8 @@ static __devexit int wm831x_gp_ldo_remove(struct platform_device *pdev)
 	struct wm831x_ldo *ldo = platform_get_drvdata(pdev);
 	struct wm831x *wm831x = ldo->wm831x;
 
+	platform_set_drvdata(pdev, NULL);
+
 	wm831x_free_irq(wm831x, platform_get_irq_byname(pdev, "UV"), ldo);
 	regulator_unregister(ldo->regulator);
 	kfree(ldo);
@@ -383,6 +385,7 @@ static struct platform_driver wm831x_gp_ldo_driver = {
 	.remove = __devexit_p(wm831x_gp_ldo_remove),
 	.driver		= {
 		.name	= "wm831x-ldo",
+		.owner	= THIS_MODULE,
 	},
 };
 
@@ -640,6 +643,7 @@ static struct platform_driver wm831x_aldo_driver = {
 	.remove = __devexit_p(wm831x_aldo_remove),
 	.driver		= {
 		.name	= "wm831x-aldo",
+		.owner	= THIS_MODULE,
 	},
 };
 
@@ -811,6 +815,7 @@ static struct platform_driver wm831x_alive_ldo_driver = {
 	.remove = __devexit_p(wm831x_alive_ldo_remove),
 	.driver		= {
 		.name	= "wm831x-alive-ldo",
+		.owner	= THIS_MODULE,
 	},
 };
 

Re: [PATCH 11/14] Regulators: wm831x-xxx - clean up driver data after removal

[Mark Brown]

On Tue, Feb 23, 2010 at 11:38:39PM -0800, Dmitry Torokhov wrote:
> It is a good tone to reset driver data after unbinding the device.
> Also set up drivers owner.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH 11/14] Regulators: wm831x-xxx - clean up driver data after removal

[Liam Girdwood]

On Tue, 2010-02-23 at 23:38 -0800, Dmitry Torokhov wrote:
> It is a good tone to reset driver data after unbinding the device.
> Also set up drivers owner.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---

Applied.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH 12/14] Regulators: wm8994 - clean up driver data after removal

[Dmitry Torokhov]

It is a good tone to reset driver data after unbinding the device.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/regulator/wm8994-regulator.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/wm8994-regulator.c b/drivers/regulator/wm8994-regulator.c
index d09e018..95454a4 100644
--- a/drivers/regulator/wm8994-regulator.c
+++ b/drivers/regulator/wm8994-regulator.c
@@ -26,7 +26,7 @@
 
 struct wm8994_ldo {
 	int enable;
-	int is_enabled;
+	bool is_enabled;
 	struct regulator_dev *regulator;
 	struct wm8994 *wm8994;
 };
@@ -43,7 +43,7 @@ static int wm8994_ldo_enable(struct regulator_dev *rdev)
 		return 0;
 
 	gpio_set_value(ldo->enable, 1);
-	ldo->is_enabled = 1;
+	ldo->is_enabled = true;
 
 	return 0;
 }
@@ -57,7 +57,7 @@ static int wm8994_ldo_disable(struct regulator_dev *rdev)
 		return -EINVAL;
 
 	gpio_set_value(ldo->enable, 0);
-	ldo->is_enabled = 0;
+	ldo->is_enabled = false;
 
 	return 0;
 }
@@ -218,7 +218,7 @@ static __devinit int wm8994_ldo_probe(struct platform_device *pdev)
 
 	ldo->wm8994 = wm8994;
 
-	ldo->is_enabled = 1;
+	ldo->is_enabled = true;
 
 	if (pdata->ldo[id].enable && gpio_is_valid(pdata->ldo[id].enable)) {
 		ldo->enable = pdata->ldo[id].enable;
@@ -263,6 +263,8 @@ static __devexit int wm8994_ldo_remove(struct platform_device *pdev)
 {
 	struct wm8994_ldo *ldo = platform_get_drvdata(pdev);
 
+	platform_set_drvdata(pdev, NULL);
+
 	regulator_unregister(ldo->regulator);
 	if (gpio_is_valid(ldo->enable))
 		gpio_free(ldo->enable);
@@ -276,6 +278,7 @@ static struct platform_driver wm8994_ldo_driver = {
 	.remove = __devexit_p(wm8994_ldo_remove),
 	.driver		= {
 		.name	= "wm8994-ldo",
+		.owner	= THIS_MODULE,
 	},
 };
 

Re: [PATCH 12/14] Regulators: wm8994 - clean up driver data after removal

[Mark Brown]

On Tue, Feb 23, 2010 at 11:38:44PM -0800, Dmitry Torokhov wrote:
> It is a good tone to reset driver data after unbinding the device.

You mean "good style" here.  Anyway,

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

BTW, this patch is an example of the sort of stuff I'm talking about
with adding additional changes that aren't documented in the
changelog.  As well as nulling out the driver data you're also...

>  struct wm8994_ldo {
>  	int enable;
> -	int is_enabled;
> +	bool is_enabled;

...doing a conversion to bool here and...

> @@ -276,6 +278,7 @@ static struct platform_driver wm8994_ldo_driver = {
>  	.remove = __devexit_p(wm8994_ldo_remove),
>  	.driver		= {
>  		.name	= "wm8994-ldo",
> +		.owner	= THIS_MODULE,
>  	},

...adding this.  The change you mention in the changelog is a single
line edit but with these extra changes the overall diffstat becomes:

>  drivers/regulator/wm8994-regulator.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)

Re: [PATCH 12/14] Regulators: wm8994 - clean up driver data after removal

[Liam Girdwood]

On Tue, 2010-02-23 at 23:38 -0800, Dmitry Torokhov wrote:
> It is a good tone to reset driver data after unbinding the device.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
> 

Applied.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH 13/14] Regulators: wm8400 - cleanup platform driver data handling

[Dmitry Torokhov]

Driver data set by platform_set_drvdata() is for private use of
the driver currently bound to teh device and not for use by parent,
subsystem and anyone else.

Also have wm8400_register_regulator() accept 'sturct wm8400 *'
instead of generic device structure.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/regulator/wm8400-regulator.c |   13 +++++++------
 include/linux/mfd/wm8400.h           |    4 +++-
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/regulator/wm8400-regulator.c b/drivers/regulator/wm8400-regulator.c
index d9a2c98..71b89e8 100644
--- a/drivers/regulator/wm8400-regulator.c
+++ b/drivers/regulator/wm8400-regulator.c
@@ -317,14 +317,17 @@ static struct regulator_desc regulators[] = {
 
 static int __devinit wm8400_regulator_probe(struct platform_device *pdev)
 {
+	struct wm8400 *wm8400 = container_of(pdev, struct wm8400, regulators[pdev->id]);
 	struct regulator_dev *rdev;
 
 	rdev = regulator_register(&regulators[pdev->id], &pdev->dev,
-		pdev->dev.platform_data, dev_get_drvdata(&pdev->dev));
+				  pdev->dev.platform_data, wm8400);
 
 	if (IS_ERR(rdev))
 		return PTR_ERR(rdev);
 
+	platform_set_drvdata(pdev, rdev);
+
 	return 0;
 }
 
@@ -332,6 +335,7 @@ static int __devexit wm8400_regulator_remove(struct platform_device *pdev)
 {
 	struct regulator_dev *rdev = platform_get_drvdata(pdev);
 
+	platform_set_drvdata(pdev, NULL);
 	regulator_unregister(rdev);
 
 	return 0;
@@ -356,11 +360,9 @@ static struct platform_driver wm8400_regulator_driver = {
  * @param reg      The regulator to control.
  * @param initdata Regulator initdata for the regulator.
  */
-int wm8400_register_regulator(struct device *dev, int reg,
+int wm8400_register_regulator(struct wm8400 *wm8400, int reg,
 			      struct regulator_init_data *initdata)
 {
-	struct wm8400 *wm8400 = dev_get_drvdata(dev);
-
 	if (wm8400->regulators[reg].name)
 		return -EBUSY;
 
@@ -368,9 +370,8 @@ int wm8400_register_regulator(struct device *dev, int reg,
 
 	wm8400->regulators[reg].name = "wm8400-regulator";
 	wm8400->regulators[reg].id = reg;
-	wm8400->regulators[reg].dev.parent = dev;
+	wm8400->regulators[reg].dev.parent = wm8400->dev;
 	wm8400->regulators[reg].dev.platform_data = initdata;
-	dev_set_drvdata(&wm8400->regulators[reg].dev, wm8400);
 
 	return platform_device_register(&wm8400->regulators[reg]);
 }
diff --git a/include/linux/mfd/wm8400.h b/include/linux/mfd/wm8400.h
index b46b566..f9e49cc 100644
--- a/include/linux/mfd/wm8400.h
+++ b/include/linux/mfd/wm8400.h
@@ -34,7 +34,9 @@ struct wm8400_platform_data {
 	int (*platform_init)(struct device *dev);
 };
 
-int wm8400_register_regulator(struct device *dev, int reg,
+struct wm8400;
+
+int wm8400_register_regulator(struct wm8400 *wm8400, int reg,
 			      struct regulator_init_data *initdata);
 
 #endif

Re: [PATCH 13/14] Regulators: wm8400 - cleanup platform driver data handling

[Mark Brown]

On Tue, Feb 23, 2010 at 11:38:50PM -0800, Dmitry Torokhov wrote:
> Driver data set by platform_set_drvdata() is for private use of
> the driver currently bound to teh device and not for use by parent,
> subsystem and anyone else.
> 
> Also have wm8400_register_regulator() accept 'sturct wm8400 *'
> instead of generic device structure.

Nack due to this change - this change would make it impossible for
callers to actually call the function.  Note that nothing including only
wm8400.h even has a struct declaration, much less defniition, for struct
wm8400.

Re: [PATCH 13/14] Regulators: wm8400 - cleanup platform driver data handling

[Dmitry Torokhov]

On Wed, Feb 24, 2010 at 10:25:49AM +0000, Mark Brown wrote:
> On Tue, Feb 23, 2010 at 11:38:50PM -0800, Dmitry Torokhov wrote:
> > Driver data set by platform_set_drvdata() is for private use of
> > the driver currently bound to teh device and not for use by parent,
> > subsystem and anyone else.
> > 
> > Also have wm8400_register_regulator() accept 'sturct wm8400 *'
> > instead of generic device structure.
> 
> Nack due to this change - this change would make it impossible for
> callers to actually call the function.  Note that nothing including only
> wm8400.h even has a struct declaration, much less defniition, for struct
> wm8400.

If you notice I added forward declaration of "struct wm8400" to wm8400.h
thus users can pass around pointer to the structure.

I really think we should refrain from passing naked 'struct device *'
pointers as much as possible since it is error prone. Otherwise
wm8400_register_regulator has no way of ensuting that passed  'dev' is
indeed wm8400.

-- 
Dmitry

Re: [PATCH 13/14] Regulators: wm8400 - cleanup platform driver data handling

[Mark Brown]

On Wed, Feb 24, 2010 at 11:02:34AM -0800, Dmitry Torokhov wrote:
> On Wed, Feb 24, 2010 at 10:25:49AM +0000, Mark Brown wrote:

> > Nack due to this change - this change would make it impossible for
> > callers to actually call the function.  Note that nothing including only
> > wm8400.h even has a struct declaration, much less defniition, for struct
> > wm8400.

> If you notice I added forward declaration of "struct wm8400" to wm8400.h
> thus users can pass around pointer to the structure.

This doesn't help unless you also provide a way for users to obtain a
struct wm8400.

> I really think we should refrain from passing naked 'struct device *'
> pointers as much as possible since it is error prone. Otherwise
> wm8400_register_regulator has no way of ensuting that passed  'dev' is
> indeed wm8400.

Right, there's no type safety here (and this whole method of registering
regulators is fairly deprecated anyway in favour of just straight
platform data) but really it doesn't buy us much - the users get exactly
the struct device they need to use passed in to them, there's really
very little chance for them to get confused about what they're talking
about.

Re: [PATCH 13/14] Regulators: wm8400 - cleanup platform driver data handling

[Dmitry Torokhov]

On Wed, Feb 24, 2010 at 07:14:03PM +0000, Mark Brown wrote:
> On Wed, Feb 24, 2010 at 11:02:34AM -0800, Dmitry Torokhov wrote:
> > On Wed, Feb 24, 2010 at 10:25:49AM +0000, Mark Brown wrote:
> 
> > > Nack due to this change - this change would make it impossible for
> > > callers to actually call the function.  Note that nothing including only
> > > wm8400.h even has a struct declaration, much less defniition, for struct
> > > wm8400.
> 
> > If you notice I added forward declaration of "struct wm8400" to wm8400.h
> > thus users can pass around pointer to the structure.
> 
> This doesn't help unless you also provide a way for users to obtain a
> struct wm8400.

Why would they need it? Only code that creates instances of wm8400 needs
to know the definition of the sturcture, the rest can simply pass the
pointer around.

I guess there is disconnect between us and I do not see any users of
wm8400_register_regulator() in linux-next... Is there another tree I
could peek at?

> 
> > I really think we should refrain from passing naked 'struct device *'
> > pointers as much as possible since it is error prone. Otherwise
> > wm8400_register_regulator has no way of ensuting that passed  'dev' is
> > indeed wm8400.
> 
> Right, there's no type safety here (and this whole method of registering
> regulators is fairly deprecated anyway in favour of just straight
> platform data) but really it doesn't buy us much - the users get exactly
> the struct device they need to use passed in to them, there's really
> very little chance for them to get confused about what they're talking
> about.

-- 
Dmitry

Re: [PATCH 13/14] Regulators: wm8400 - cleanup platform driver data handling

[Mark Brown]

On Wed, Feb 24, 2010 at 11:21:26AM -0800, Dmitry Torokhov wrote:
> On Wed, Feb 24, 2010 at 07:14:03PM +0000, Mark Brown wrote:

> > This doesn't help unless you also provide a way for users to obtain a
> > struct wm8400.

> Why would they need it? Only code that creates instances of wm8400 needs
> to know the definition of the sturcture, the rest can simply pass the
> pointer around.

> I guess there is disconnect between us and I do not see any users of
> wm8400_register_regulator() in linux-next... Is there another tree I
> could peek at?

There are no users in mainline.  This would be called by board specific
code from the init callback of the wm8400 - you'd need to pass that
callback the struct wm8400.

In any case, this is clearly an unrelated change to whatever else you
were doing to the driver so should be split off into a separate patch,
but if this is being changed at all then it'd be much more sensible to
change it to use a more modern pattern which completely removes the
wm8400_register_regulator() function and just uses platform data.

Re: [PATCH 13/14] Regulators: wm8400 - cleanup platform driver data handling

[Dmitry Torokhov]

On Wed, Feb 24, 2010 at 08:40:56PM +0000, Mark Brown wrote:
> On Wed, Feb 24, 2010 at 11:21:26AM -0800, Dmitry Torokhov wrote:
> > On Wed, Feb 24, 2010 at 07:14:03PM +0000, Mark Brown wrote:
> 
> > > This doesn't help unless you also provide a way for users to obtain a
> > > struct wm8400.
> 
> > Why would they need it? Only code that creates instances of wm8400 needs
> > to know the definition of the sturcture, the rest can simply pass the
> > pointer around.
> 
> > I guess there is disconnect between us and I do not see any users of
> > wm8400_register_regulator() in linux-next... Is there another tree I
> > could peek at?
> 
> There are no users in mainline.  This would be called by board specific
> code from the init callback of the wm8400 - you'd need to pass that
> callback the struct wm8400.
> 
> In any case, this is clearly an unrelated change to whatever else you
> were doing to the driver so should be split off into a separate patch,
> but if this is being changed at all then it'd be much more sensible to
> change it to use a more modern pattern which completely removes the
> wm8400_register_regulator() function and just uses platform data.

Fair enough, I removed the offending part, updated patch below.

-- 
Dmitry

regulator: wm8400 - cleanup platform driver data handling

Driver data set by platform_set_drvdata() is for private use of
the driver currently bound to teh device and not for use by parent,
subsystem and anyone else.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/regulator/wm8400-regulator.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)


diff --git a/drivers/regulator/wm8400-regulator.c b/drivers/regulator/wm8400-regulator.c
index d9a2c98..924c7eb 100644
--- a/drivers/regulator/wm8400-regulator.c
+++ b/drivers/regulator/wm8400-regulator.c
@@ -317,14 +317,17 @@ static struct regulator_desc regulators[] = {
 
 static int __devinit wm8400_regulator_probe(struct platform_device *pdev)
 {
+	struct wm8400 *wm8400 = container_of(pdev, struct wm8400, regulators[pdev->id]);
 	struct regulator_dev *rdev;
 
 	rdev = regulator_register(&regulators[pdev->id], &pdev->dev,
-		pdev->dev.platform_data, dev_get_drvdata(&pdev->dev));
+				  pdev->dev.platform_data, wm8400);
 
 	if (IS_ERR(rdev))
 		return PTR_ERR(rdev);
 
+	platform_set_drvdata(pdev, rdev);
+
 	return 0;
 }
 
@@ -332,6 +335,7 @@ static int __devexit wm8400_regulator_remove(struct platform_device *pdev)
 {
 	struct regulator_dev *rdev = platform_get_drvdata(pdev);
 
+	platform_set_drvdata(pdev, NULL);
 	regulator_unregister(rdev);
 
 	return 0;
@@ -370,7 +374,6 @@ int wm8400_register_regulator(struct device *dev, int reg,
 	wm8400->regulators[reg].id = reg;
 	wm8400->regulators[reg].dev.parent = dev;
 	wm8400->regulators[reg].dev.platform_data = initdata;
-	dev_set_drvdata(&wm8400->regulators[reg].dev, wm8400);
 
 	return platform_device_register(&wm8400->regulators[reg]);
 }

Re: [PATCH 13/14] Regulators: wm8400 - cleanup platform driver data handling

[Mark Brown]

On Thu, Feb 25, 2010 at 01:55:37AM -0800, Dmitry Torokhov wrote:

> Fair enough, I removed the offending part, updated patch below.

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

> -- 
> Dmitry
> 
> regulator: wm8400 - cleanup platform driver data handling

but note that including the patch after your sig separator means that a
lot of MUAs will hide the patch from users which makes it a bit hard to
read.

Re: [PATCH 13/14] Regulators: wm8400 - cleanup platform driver data handling

[Liam Girdwood]

On Thu, 2010-02-25 at 01:55 -0800, Dmitry Torokhov wrote:
> regulator: wm8400 - cleanup platform driver data handling
> 
> Driver data set by platform_set_drvdata() is for private use of
> the driver currently bound to teh device and not for use by parent,
> subsystem and anyone else.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
> 
>  drivers/regulator/wm8400-regulator.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 

Applied.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH 14/14] Regulators: max8925-regulator - clean up driver data after removal

[Dmitry Torokhov]

It is a good tone to reset driver data after unbinding the device.
Also change find_regulator_info() fro inline to __devinit - let compiler
figure out if it wants it to be inlined or not.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/regulator/max8925-regulator.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/max8925-regulator.c b/drivers/regulator/max8925-regulator.c
index 67873f0..b6218f1 100644
--- a/drivers/regulator/max8925-regulator.c
+++ b/drivers/regulator/max8925-regulator.c
@@ -230,7 +230,7 @@ static struct max8925_regulator_info max8925_regulator_info[] = {
 	MAX8925_LDO(20, 750, 3900, 50),
 };
 
-static inline struct max8925_regulator_info *find_regulator_info(int id)
+static struct max8925_regulator_info * __devinit find_regulator_info(int id)
 {
 	struct max8925_regulator_info *ri;
 	int i;
@@ -247,7 +247,7 @@ static int __devinit max8925_regulator_probe(struct platform_device *pdev)
 {
 	struct max8925_chip *chip = dev_get_drvdata(pdev->dev.parent);
 	struct max8925_platform_data *pdata = chip->dev->platform_data;
-	struct max8925_regulator_info *ri = NULL;
+	struct max8925_regulator_info *ri;
 	struct regulator_dev *rdev;
 
 	ri = find_regulator_info(pdev->id);
@@ -274,7 +274,9 @@ static int __devexit max8925_regulator_remove(struct platform_device *pdev)
 {
 	struct regulator_dev *rdev = platform_get_drvdata(pdev);
 
+	platform_set_drvdata(pdev, NULL);
 	regulator_unregister(rdev);
+
 	return 0;
 }
 

Re: [PATCH 14/14] Regulators: max8925-regulator - clean up driver data after removal

[Mark Brown]

On Tue, Feb 23, 2010 at 11:38:55PM -0800, Dmitry Torokhov wrote:
> It is a good tone to reset driver data after unbinding the device.
> Also change find_regulator_info() fro inline to __devinit - let compiler
> figure out if it wants it to be inlined or not.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH 14/14] Regulators: max8925-regulator - clean up driver data after removal

[Liam Girdwood]

On Tue, 2010-02-23 at 23:38 -0800, Dmitry Torokhov wrote:
> It is a good tone to reset driver data after unbinding the device.
> Also change find_regulator_info() fro inline to __devinit - let compiler
> figure out if it wants it to be inlined or not.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Applied.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 00/14] Assorted small patches for regulators

[Mark Brown]

On Tue, Feb 23, 2010 at 11:37:39PM -0800, Dmitry Torokhov wrote:

> I happend to peek into drivers/regulator and saw a bunch of small issues, so
> here goes. The patches are against linux-next, compile-tested only since I
> don't have the hardware,

I'm working through these now, but a few general issues:

 - It'd be very much easier to review patches that do one thing at once,
   especially when the patches do things more invasive than just adding
   annotations or changing types.  The random cleanup stuff makes the
   invasive changes much harder to see and review.
 - Frequently your patches include additional changes above those that
   you list in the changelog which again increases the effort require to
   reviwe - things like random whitespace changes and the addition of
   module annotations seem particularly prone to this.
 - Please always use a subject line for your patches which fits the
   style of the subsystem.  You're using "Regulators:" as a prefix when
   pretty much everything else uses "regulator:".