[PATCH 00/14] Assorted small patches for regulators
Dmitry Torokhov
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(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Dmitry Torokhov
to reset fields in structures that are about to be freed.
Signed-off-by: Dmitry Torokhov <dt...@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;
Dmitry Torokhov
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 <dt...@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;
Dmitry Torokhov
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 <dt...@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)
Dmitry Torokhov
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 <dt...@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(®ulators[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
Dmitry Torokhov
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 <dt...@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,
},
};
Dmitry Torokhov
---
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;
Mark Brown
> 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:".
Mark Brown
> 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 <dt...@mail.ru>
Acked-by: Mark Brown <bro...@opensource.wolfsonmicro.com>
Mark Brown
> Signed-off-by: Dmitry Torokhov <dt...@mail.ru>
Acked-by: Mark Brown <bro...@opensource.wolfsonmicro.com>
Mark Brown
> 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.
Mark Brown
> 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 <dt...@mail.ru>
Acked-by: Mark Brown <bro...@opensource.wolfsonmicro.com>
Mark Brown
> 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 <dt...@mail.ru>
Acked-by: Mark Brown <bro...@opensource.wolfsonmicro.com>
Mark Brown
> 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 <dt...@mail.ru>
Acked-by: Mark Brown <bro...@opensource.wolfsonmicro.com>
Dmitry Torokhov
> 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
Mark Brown
> 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.
Dmitry Torokhov
> 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
Mark Brown
> 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.
Dmitry Torokhov
> 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 <dt...@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(®ulators[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]);
}
Liam Girdwood
> 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 <dt...@mail.ru>
Applied.
Thanks
Liam
--
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk
Liam Girdwood
> 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 <dt...@mail.ru>
> ---
>
Applied.
Thanks
Liam
--
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk
--
Liam Girdwood
> ---
>
> 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
--
Liam Girdwood
> 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 <dt...@mail.ru>
Applied.
Thanks
Liam
--
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk
--
Liam Girdwood
> 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 <dt...@mail.ru>
Applied.
Thanks
Liam
--
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk
--
Mark Brown
> Fair enough, I removed the offending part, updated patch below.
Acked-by: Mark Brown <bro...@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.
Liam Girdwood
> 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 <dt...@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
--