regulator: core: Add checking n_voltages if using list_voltage() to read voltage regulators

PATCH In review Axel Lin 2 participants 12 messages Aug 10, 2012 – Aug 13, 2012

Subsystems (2)

OMAP2+ support Voltage and current regulator framework

lore

Share & tools

lore

Axel Lin

author

2012-08-10 01:32 UTC #0 lore raw

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(-)

drivers/regulator/core.c +2 -1

index 0fffeae..5469f9f 100644
@@ -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;

Signature · 1 line

1.7.9.5

Axel Lin

author

2012-08-10 01:33 UTC #1 lore raw

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(-)

drivers/regulator/ab3100.c +0 -6

index c151fd5..65ad2b3 100644
@@ -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 = {

Signature · 1 line

1.7.9.5

Mark Brown

2012-08-13 12:31 UTC #2 lore raw

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

Quoted 2 lines · Axel Lin

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

applied 2-6.

Axel Lin

author

2012-08-10 01:35 UTC #3 lore raw

PATCH 3/6 regulator: ab8500: Remove get_voltage implementation for ab8500_regulator_fixed_ops

In reply to Axel Lin

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(-)

drivers/regulator/ab8500.c +0 -6

index 8807166..c884a5c 100644
@@ -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,
 };

Signature · 1 line

1.7.9.5

Axel Lin

author

2012-08-10 01:36 UTC #4 lore raw

PATCH 4/6 regulator: isl6271a: Remove get_voltage implementation for isl_fixed_ops

In reply to Axel Lin

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(-)

drivers/regulator/isl6271a-regulator.c +0 -6

index 1d145a0..d8ecf49 100644
@@ -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,
 };

Signature · 1 line

1.7.9.5

Axel Lin

author

2012-08-10 01:37 UTC #5 lore raw

PATCH 5/6 regulator: twl: Remove get_voltage implementation for single voltage regulators

In reply to Axel Lin

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(-)

drivers/regulator/twl-regulator.c +0 ~~-11~~

index f2f49e2..2d9a2a8 100644
@@ -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,

Signature · 1 line

1.7.9.5

Axel Lin

author

2012-08-10 01:38 UTC #6 lore raw

PATCH 6/6 regulator: mc13xxx: Remove get_voltage implementation for single voltage regulators

In reply to Axel Lin

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(-)

drivers/regulator/mc13783-regulator.c +0 -1

index 2587ea1..4977b19 100644
@@ -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)

drivers/regulator/mc13892-regulator.c +0 -1

index 09265f0..1fa6381 100644
@@ -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)

drivers/regulator/mc13xxx-regulator-core.c +0 ~~-11~~

index 8151889..88cbb83 100644
@@ -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);

drivers/regulator/mc13xxx.h +0 -1

index eaff551..06c8903 100644
@@ -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);

Signature · 1 line

1.7.9.5

Mark Brown

2012-08-10 10:05 UTC #7 lore raw

In reply to Axel Lin

Quoted 2 lines · Axel Lin

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.

Axel Lin

author

2012-08-10 12:27 UTC #8 lore raw

2012/8/10 Mark Brown <[email protected]>:

Quoted 6 lines · Axel Lin

Quoted 2 lines
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.

Mark Brown

2012-08-10 12:29 UTC #9 lore raw

Quoted 8 lines · Axel Lin

2012/8/10 Mark Brown <[email protected]>:
Quoted 2 lines
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.

Axel Lin

author

2012-08-11 02:23 UTC #10 lore raw

2012/8/10 Mark Brown <[email protected]>:

Quoted 6 lines · Axel Lin

Quoted 4 lines
2012/8/10 Mark Brown <[email protected]>:
Quoted 2 lines
We should be failing to register these regulators in the first place, or at least complaining extremely loudly about them.

Quoted 6 lines

Quoted 3 lines
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,
};

Mark Brown

2012-08-13 12:30 UTC #11 lore raw

Quoted 1 line · Axel Lin

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

They should have a read operation implemented.