2012-08-10 01:32:43

by Axel Lin

[permalink] [raw]
Subject: [PATCH 1/6] regulator: core: Add checking n_voltages if using list_voltage() to read voltage regulators

Use list_voltage() to read single voltage regulators should be only applied to
single voltage regulators, thus add checking n_voltages for this case.

Signed-off-by: Axel Lin <[email protected]>
---
drivers/regulator/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 0fffeae..5469f9f 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2395,7 +2395,8 @@ static int _regulator_get_voltage(struct regulator_dev *rdev)
ret = rdev->desc->ops->list_voltage(rdev, sel);
} else if (rdev->desc->ops->get_voltage) {
ret = rdev->desc->ops->get_voltage(rdev);
- } else if (rdev->desc->ops->list_voltage) {
+ } else if (rdev->desc->ops->list_voltage &&
+ (rdev->desc->n_voltages == 1)) {
ret = rdev->desc->ops->list_voltage(rdev, 0);
} else {
return -EINVAL;
--
1.7.9.5



2012-08-10 01:33:52

by Axel Lin

[permalink] [raw]
Subject: [PATCH 2/6] regulator: ab3100: Remove get_voltage implementation for regulator_ops_fixed

This is not required after commit f7df20ec
"regulator: core: Use list_voltage() to read single voltage regulators"

Signed-off-by: Axel Lin <[email protected]>
---
drivers/regulator/ab3100.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/drivers/regulator/ab3100.c b/drivers/regulator/ab3100.c
index c151fd5..65ad2b3 100644
--- a/drivers/regulator/ab3100.c
+++ b/drivers/regulator/ab3100.c
@@ -347,17 +347,11 @@ static int ab3100_get_voltage_regulator_external(struct regulator_dev *reg)
return abreg->plfdata->external_voltage;
}

-static int ab3100_get_fixed_voltage_regulator(struct regulator_dev *reg)
-{
- return reg->desc->min_uV;
-}
-
static struct regulator_ops regulator_ops_fixed = {
.list_voltage = regulator_list_voltage_linear,
.enable = ab3100_enable_regulator,
.disable = ab3100_disable_regulator,
.is_enabled = ab3100_is_enabled_regulator,
- .get_voltage = ab3100_get_fixed_voltage_regulator,
};

static struct regulator_ops regulator_ops_variable = {
--
1.7.9.5


2012-08-10 01:35:00

by Axel Lin

[permalink] [raw]
Subject: [PATCH 3/6] regulator: ab8500: Remove get_voltage implementation for ab8500_regulator_fixed_ops

This is not required after commit f7df20ec
"regulator: core: Use list_voltage() to read single voltage regulators"

Signed-off-by: Axel Lin <[email protected]>
---
drivers/regulator/ab8500.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/drivers/regulator/ab8500.c b/drivers/regulator/ab8500.c
index 8807166..c884a5c 100644
--- a/drivers/regulator/ab8500.c
+++ b/drivers/regulator/ab8500.c
@@ -257,16 +257,10 @@ static struct regulator_ops ab8500_regulator_ops = {
.set_voltage_time_sel = ab8500_regulator_set_voltage_time_sel,
};

-static int ab8500_fixed_get_voltage(struct regulator_dev *rdev)
-{
- return rdev->desc->min_uV;
-}
-
static struct regulator_ops ab8500_regulator_fixed_ops = {
.enable = ab8500_regulator_enable,
.disable = ab8500_regulator_disable,
.is_enabled = ab8500_regulator_is_enabled,
- .get_voltage = ab8500_fixed_get_voltage,
.list_voltage = regulator_list_voltage_linear,
};

--
1.7.9.5


2012-08-10 01:36:05

by Axel Lin

[permalink] [raw]
Subject: [PATCH 4/6] regulator: isl6271a: Remove get_voltage implementation for isl_fixed_ops

This is not required after commit f7df20ec
"regulator: core: Use list_voltage() to read single voltage regulators"

Signed-off-by: Axel Lin <[email protected]>
---
drivers/regulator/isl6271a-regulator.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/drivers/regulator/isl6271a-regulator.c b/drivers/regulator/isl6271a-regulator.c
index 1d145a0..d8ecf49 100644
--- a/drivers/regulator/isl6271a-regulator.c
+++ b/drivers/regulator/isl6271a-regulator.c
@@ -73,13 +73,7 @@ static struct regulator_ops isl_core_ops = {
.map_voltage = regulator_map_voltage_linear,
};

-static int isl6271a_get_fixed_voltage(struct regulator_dev *dev)
-{
- return dev->desc->min_uV;
-}
-
static struct regulator_ops isl_fixed_ops = {
- .get_voltage = isl6271a_get_fixed_voltage,
.list_voltage = regulator_list_voltage_linear,
};

--
1.7.9.5


2012-08-10 01:37:17

by Axel Lin

[permalink] [raw]
Subject: [PATCH 5/6] regulator: twl: Remove get_voltage implementation for single voltage regulators

This is not required after commit f7df20ec
"regulator: core: Use list_voltage() to read single voltage regulators"

Signed-off-by: Axel Lin <[email protected]>
---
drivers/regulator/twl-regulator.c | 11 -----------
1 file changed, 11 deletions(-)

diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
index f2f49e2..2d9a2a8 100644
--- a/drivers/regulator/twl-regulator.c
+++ b/drivers/regulator/twl-regulator.c
@@ -624,18 +624,9 @@ static int twlfixed_list_voltage(struct regulator_dev *rdev, unsigned index)
return info->min_mV * 1000;
}

-static int twlfixed_get_voltage(struct regulator_dev *rdev)
-{
- struct twlreg_info *info = rdev_get_drvdata(rdev);
-
- return info->min_mV * 1000;
-}
-
static struct regulator_ops twl4030fixed_ops = {
.list_voltage = twlfixed_list_voltage,

- .get_voltage = twlfixed_get_voltage,
-
.enable = twl4030reg_enable,
.disable = twl4030reg_disable,
.is_enabled = twl4030reg_is_enabled,
@@ -648,8 +639,6 @@ static struct regulator_ops twl4030fixed_ops = {
static struct regulator_ops twl6030fixed_ops = {
.list_voltage = twlfixed_list_voltage,

- .get_voltage = twlfixed_get_voltage,
-
.enable = twl6030reg_enable,
.disable = twl6030reg_disable,
.is_enabled = twl6030reg_is_enabled,
--
1.7.9.5


2012-08-10 01:38:47

by Axel Lin

[permalink] [raw]
Subject: [PATCH 6/6] regulator: mc13xxx: Remove get_voltage implementation for single voltage regulators

This is not required after commit f7df20ec
"regulator: core: Use list_voltage() to read single voltage regulators"

Signed-off-by: Axel Lin <[email protected]>
---
drivers/regulator/mc13783-regulator.c | 1 -
drivers/regulator/mc13892-regulator.c | 1 -
drivers/regulator/mc13xxx-regulator-core.c | 11 -----------
drivers/regulator/mc13xxx.h | 1 -
4 files changed, 14 deletions(-)

diff --git a/drivers/regulator/mc13783-regulator.c b/drivers/regulator/mc13783-regulator.c
index 2587ea1..4977b19 100644
--- a/drivers/regulator/mc13783-regulator.c
+++ b/drivers/regulator/mc13783-regulator.c
@@ -324,7 +324,6 @@ static struct regulator_ops mc13783_gpo_regulator_ops = {
.is_enabled = mc13783_gpo_regulator_is_enabled,
.list_voltage = regulator_list_voltage_table,
.set_voltage = mc13xxx_fixed_regulator_set_voltage,
- .get_voltage = mc13xxx_fixed_regulator_get_voltage,
};

static int __devinit mc13783_regulator_probe(struct platform_device *pdev)
diff --git a/drivers/regulator/mc13892-regulator.c b/drivers/regulator/mc13892-regulator.c
index 09265f0..1fa6381 100644
--- a/drivers/regulator/mc13892-regulator.c
+++ b/drivers/regulator/mc13892-regulator.c
@@ -390,7 +390,6 @@ static struct regulator_ops mc13892_gpo_regulator_ops = {
.is_enabled = mc13892_gpo_regulator_is_enabled,
.list_voltage = regulator_list_voltage_table,
.set_voltage = mc13xxx_fixed_regulator_set_voltage,
- .get_voltage = mc13xxx_fixed_regulator_get_voltage,
};

static int mc13892_sw_regulator_get_voltage_sel(struct regulator_dev *rdev)
diff --git a/drivers/regulator/mc13xxx-regulator-core.c b/drivers/regulator/mc13xxx-regulator-core.c
index 8151889..88cbb83 100644
--- a/drivers/regulator/mc13xxx-regulator-core.c
+++ b/drivers/regulator/mc13xxx-regulator-core.c
@@ -152,23 +152,12 @@ int mc13xxx_fixed_regulator_set_voltage(struct regulator_dev *rdev, int min_uV,
}
EXPORT_SYMBOL_GPL(mc13xxx_fixed_regulator_set_voltage);

-int mc13xxx_fixed_regulator_get_voltage(struct regulator_dev *rdev)
-{
- int id = rdev_get_id(rdev);
-
- dev_dbg(rdev_get_dev(rdev), "%s id: %d\n", __func__, id);
-
- return rdev->desc->volt_table[0];
-}
-EXPORT_SYMBOL_GPL(mc13xxx_fixed_regulator_get_voltage);
-
struct regulator_ops mc13xxx_fixed_regulator_ops = {
.enable = mc13xxx_regulator_enable,
.disable = mc13xxx_regulator_disable,
.is_enabled = mc13xxx_regulator_is_enabled,
.list_voltage = regulator_list_voltage_table,
.set_voltage = mc13xxx_fixed_regulator_set_voltage,
- .get_voltage = mc13xxx_fixed_regulator_get_voltage,
};
EXPORT_SYMBOL_GPL(mc13xxx_fixed_regulator_ops);

diff --git a/drivers/regulator/mc13xxx.h b/drivers/regulator/mc13xxx.h
index eaff551..06c8903 100644
--- a/drivers/regulator/mc13xxx.h
+++ b/drivers/regulator/mc13xxx.h
@@ -34,7 +34,6 @@ struct mc13xxx_regulator_priv {

extern int mc13xxx_fixed_regulator_set_voltage(struct regulator_dev *rdev,
int min_uV, int max_uV, unsigned *selector);
-extern int mc13xxx_fixed_regulator_get_voltage(struct regulator_dev *rdev);

#ifdef CONFIG_OF
extern int mc13xxx_get_num_regulators_dt(struct platform_device *pdev);
--
1.7.9.5


2012-08-10 10:05:52

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/6] regulator: core: Add checking n_voltages if using list_voltage() to read voltage regulators

On Fri, Aug 10, 2012 at 09:32:33AM +0800, Axel Lin wrote:
> Use list_voltage() to read single voltage regulators should be only applied to
> single voltage regulators, thus add checking n_voltages for this case.

We should be failing to register these regulators in the first place, or
at least complaining extremely loudly about them.

2012-08-10 12:27:54

by Axel Lin

[permalink] [raw]
Subject: Re: [PATCH 1/6] regulator: core: Add checking n_voltages if using list_voltage() to read voltage regulators

2012/8/10 Mark Brown <[email protected]>:
> On Fri, Aug 10, 2012 at 09:32:33AM +0800, Axel Lin wrote:
>> Use list_voltage() to read single voltage regulators should be only applied to
>> single voltage regulators, thus add checking n_voltages for this case.
>
> We should be failing to register these regulators in the first place, or
> at least complaining extremely loudly about them.

Oh. My original intention is to prevent using list_voltage() to read
voltage regulators for the case "n_voltages > 1" in case of both get_voltage
and get_voltage_sel are not implemented.

2012-08-10 12:29:11

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/6] regulator: core: Add checking n_voltages if using list_voltage() to read voltage regulators

On Fri, Aug 10, 2012 at 08:27:32PM +0800, Axel Lin wrote:
> 2012/8/10 Mark Brown <[email protected]>:

> > We should be failing to register these regulators in the first place, or
> > at least complaining extremely loudly about them.

> Oh. My original intention is to prevent using list_voltage() to read
> voltage regulators for the case "n_voltages > 1" in case of both get_voltage
> and get_voltage_sel are not implemented.

Yes, I see the intention - what I'm saying is that a regulator like that
makes no sense in the first place.

2012-08-11 02:23:16

by Axel Lin

[permalink] [raw]
Subject: Re: [PATCH 1/6] regulator: core: Add checking n_voltages if using list_voltage() to read voltage regulators

2012/8/10 Mark Brown <[email protected]>:
> On Fri, Aug 10, 2012 at 08:27:32PM +0800, Axel Lin wrote:
>> 2012/8/10 Mark Brown <[email protected]>:
>
>> > We should be failing to register these regulators in the first place, or
>> > at least complaining extremely loudly about them.
>
>> Oh. My original intention is to prevent using list_voltage() to read
>> voltage regulators for the case "n_voltages > 1" in case of both get_voltage
>> and get_voltage_sel are not implemented.
>
> Yes, I see the intention - what I'm saying is that a regulator like that
> makes no sense in the first place.

We do have such case in drivers/regulator/max1586.c

/*
* The Maxim 1586 controls V3 and V6 voltages, but offers no way of reading back
* the set up value.
*/
static struct regulator_ops max1586_v3_ops = {
.set_voltage_sel = max1586_v3_set_voltage_sel,
.list_voltage = regulator_list_voltage_linear,
.map_voltage = regulator_map_voltage_linear,
};

static struct regulator_ops max1586_v6_ops = {
.set_voltage_sel = max1586_v6_set_voltage_sel,
.list_voltage = regulator_list_voltage_table,
};

2012-08-13 12:30:47

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/6] regulator: core: Add checking n_voltages if using list_voltage() to read voltage regulators

On Sat, Aug 11, 2012 at 10:22:54AM +0800, Axel Lin wrote:

> We do have such case in drivers/regulator/max1586.c

They should have a read operation implemented.

2012-08-13 12:31:35

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 2/6] regulator: ab3100: Remove get_voltage implementation for regulator_ops_fixed

On Fri, Aug 10, 2012 at 09:33:44AM +0800, Axel Lin wrote:
> This is not required after commit f7df20ec
> "regulator: core: Use list_voltage() to read single voltage regulators"

applied 2-6.