On 04/10/2013 08:40 AM, Axel Lin wrote:
When setting voltage for AB8540_LDO_AUX3, current code only updates one of
info->voltage_reg and info->expand_register registers which is wrong.
To ensure we set to correct voltage, it always needs to clear or set
expand_register.voltage_mask bit of expand_register.
The original code is wrong. It is not possible to leave 3.05 V once set.
The function of the expand register bit is the following (from the user
manual):
0: VAUX3 output voltage is determined by Vaux3Sel bit settings in
register VldoCVaux3Sel
1: VAUX3 output voltage is set to 3.05 V regardless of Vaux3Sel settings
in register VldoCVaux3Sel
(VldoCVaux3Sel is the register at 0x0421)
So when going to 3.05 V I would like to set the bit as you are doing.
But when leaving 3.05 V for another voltage I would prefer setting the
target voltage before clearing the expand register bit.
Signed-off-by: Axel Lin <axel....@ingics.com>
---
drivers/regulator/ab8500.c | 55 ++++++++++++++++++++++++++++----------------
1 file changed, 35 insertions(+), 20 deletions(-)
diff --git a/drivers/regulator/ab8500.c b/drivers/regulator/ab8500.c
index 9ebd131..222a63e 100644
--- a/drivers/regulator/ab8500.c
+++ b/drivers/regulator/ab8500.c
@@ -605,39 +605,54 @@ static int ab8540_aux3_regulator_set_voltage_sel(struct
regulator_dev *rdev,
{
int ret;
struct ab8500_regulator_info *info = rdev_get_drvdata(rdev);
- u8 regval;
+ u8 regval, regval_expand;
if (info == NULL) {
dev_err(rdev_get_dev(rdev), "regulator info null pointer\n");
return -EINVAL;
}
- if (selector >= info->expand_register.voltage_limit) {
- /* Vaux3 bit4 has different layout */
- regval = (u8)selector << info->expand_register.voltage_shift;
- ret = abx500_mask_and_set_register_interruptible(info->dev,
- info->expand_register.voltage_bank,
- info->expand_register.voltage_reg,
- info->expand_register.voltage_mask,
- regval);
- } else {
- /* set the registers for the request */
- regval = (u8)selector << info->voltage_shift;
- ret = abx500_mask_and_set_register_interruptible(info->dev,
+ if (selector >= info->expand_register.voltage_limit)
+ regval_expand = info->expand_register.voltage_mask;
+ else
+ regval_expand = 0;
+
+ ret = abx500_mask_and_set_register_interruptible(info->dev,
+ info->expand_register.voltage_bank,
+ info->expand_register.voltage_reg,
+ info->expand_register.voltage_mask,
+ regval_expand);
+ if (ret < 0) {
+ dev_err(rdev_get_dev(rdev),
+ "couldn't set expand voltage reg for regulator\n");
+ return ret;
+ }
+
+ dev_vdbg(rdev_get_dev(rdev),
+ "%s-set_voltage expand (bank, reg, mask, value): 0x%x, 0x%x, 0x%x,
0x%x\n",
+ info->desc.name, info->expand_register.voltage_bank,
+ info->expand_register.voltage_reg,
+ info->expand_register.voltage_mask, regval_expand);
+
+ if (selector >= info->expand_register.voltage_limit)
+ return 0;
+
+ regval = (u8)selector << info->voltage_shift;
+ ret = abx500_mask_and_set_register_interruptible(info->dev,
info->voltage_bank, info->voltage_reg,
info->voltage_mask, regval);
- }
- if (ret < 0)
+ if (ret < 0) {
dev_err(rdev_get_dev(rdev),
"couldn't set voltage reg for regulator\n");
+ return ret;
+ }
dev_vdbg(rdev_get_dev(rdev),
- "%s-set_voltage (bank, reg, mask, value): 0x%x, 0x%x,
0x%x,"
- " 0x%x\n",
- info->desc.name, info->voltage_bank, info->voltage_reg,
- info->voltage_mask, regval);
+ "%s-set_voltage (bank, reg, mask, value): 0x%x, 0x%x, 0x%x,
0x%x\n",
+ info->desc.name, info->voltage_bank, info->voltage_reg,
+ info->voltage_mask, regval);
- return ret;
+ return 0;
}
static struct regulator_ops ab8500_regulator_volt_mode_ops = {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
