regulator: core: Allow specify apply_[reg|bit] for regmap based voltage_sel operations

PATCH In review Axel Lin 5 participants 10 messages Dec 18, 2012 – Oct 2, 2013

Subsystems (2)

Voltage and current regulator framework Dialog semiconductor drivers

lore

Share & tools

lore

Axel Lin

author

2012-12-18 01:30 UTC #0 lore raw

PATCH 1/4 regulator: core: Allow specify apply_[reg|bit] for regmap based voltage_sel operations

Some DVM regulators needs to update apply_bit after setting vsel_reg to initiate voltage change on the output. This patch adds apply_reg and apply_bit to struct regulator_desc and update regulator_set_voltage_sel_regmap() to set apply_bit of apply_reg when apply_bit is set.

Signed-off-by Axel Lin <[email protected]>

---
 drivers/regulator/core.c         |   12 +++++++++++-
 include/linux/regulator/driver.h |    6 ++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

drivers/regulator/core.c +11 -1

index a35858e..9afcdda3 100644
@@ -2074,10 +2074,20 @@ EXPORT_SYMBOL_GPL(regulator_get_voltage_sel_regmap);
  */
 int regulator_set_voltage_sel_regmap(struct regulator_dev *rdev, unsigned sel)
 {
+	int ret;
+
 	sel <<= ffs(rdev->desc->vsel_mask) - 1;
 
-	return regmap_update_bits(rdev->regmap, rdev->desc->vsel_reg,
+	ret = regmap_update_bits(rdev->regmap, rdev->desc->vsel_reg,
 				  rdev->desc->vsel_mask, sel);
+	if (ret)
+		return ret;
+
+	if (rdev->desc->apply_bit)
+		ret = regmap_update_bits(rdev->regmap, rdev->desc->apply_reg,
+					 rdev->desc->apply_bit,
+					 rdev->desc->apply_bit);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(regulator_set_voltage_sel_regmap);

include/linux/regulator/driver.h +6 -0

index d10bb0f..23070fd 100644
@@ -193,6 +193,10 @@ enum regulator_type {
  *
  * @vsel_reg: Register for selector when using regulator_regmap_X_voltage_
  * @vsel_mask: Mask for register bitfield used for selector
+ * @apply_reg: Register for initiate voltage change on the output when
+ *                using regulator_set_voltage_sel_regmap
+ * @apply_bit: Register bitfield used for initiate voltage change on the
+ *                output when using regulator_set_voltage_sel_regmap
  * @enable_reg: Register for control when using regmap enable/disable ops
  * @enable_mask: Mask for control when using regmap enable/disable ops
  *

@@ -218,6 +222,8 @@ struct regulator_desc {
 
 	unsigned int vsel_reg;
 	unsigned int vsel_mask;
+	unsigned int apply_reg;
+	unsigned int apply_bit;
 	unsigned int enable_reg;
 	unsigned int enable_mask;
 	unsigned int bypass_reg;

Signature · 1 line

1.7.9.5

Axel Lin

author

2012-12-18 01:37 UTC #1 lore raw

PATCH 2/4 regulator: 88pm8607: Use apply_[reg|bit] with regmap based voltage_sel operations

Signed-off-by Axel Lin <[email protected]>

---
 drivers/regulator/88pm8607.c |   43 ++++++++++++------------------------------
 1 file changed, 12 insertions(+), 31 deletions(-)

drivers/regulator/88pm8607.c +12 ~~-31~~

index 2b55711..9ebdf8c 100644
@@ -30,8 +30,6 @@ struct pm8607_regulator_info {
 	unsigned int	*vol_table;
 	unsigned int	*vol_suspend;
 
-	int	update_reg;
-	int	update_bit;
 	int	slope_double;
 };

@@ -222,29 +220,6 @@ static int pm8607_list_voltage(struct regulator_dev *rdev, unsigned index)
 	return ret;
 }
 
-static int pm8607_set_voltage_sel(struct regulator_dev *rdev, unsigned selector)
-{
-	struct pm8607_regulator_info *info = rdev_get_drvdata(rdev);
-	uint8_t val;
-	int ret;
-
-	val = (uint8_t)(selector << (ffs(rdev->desc->vsel_mask) - 1));
-
-	ret = pm860x_set_bits(info->i2c, rdev->desc->vsel_reg,
-			      rdev->desc->vsel_mask, val);
-	if (ret)
-		return ret;
-	switch (info->desc.id) {
-	case PM8607_ID_BUCK1:
-	case PM8607_ID_BUCK3:
-		ret = pm860x_set_bits(info->i2c, info->update_reg,
-				      1 << info->update_bit,
-				      1 << info->update_bit);
-		break;
-	}
-	return ret;
-}
-
 static int pm8606_preg_enable(struct regulator_dev *rdev)
 {
 	struct pm8607_regulator_info *info = rdev_get_drvdata(rdev);

@@ -276,7 +251,7 @@ static int pm8606_preg_is_enabled(struct regulator_dev *rdev)
 
 static struct regulator_ops pm8607_regulator_ops = {
 	.list_voltage	= pm8607_list_voltage,
-	.set_voltage_sel = pm8607_set_voltage_sel,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,

@@ -313,11 +288,11 @@ static struct regulator_ops pm8606_preg_ops = {
 		.n_voltages = ARRAY_SIZE(vreg##_table),			\
 		.vsel_reg = PM8607_##vreg,				\
 		.vsel_mask = ARRAY_SIZE(vreg##_table) - 1,		\
+		.apply_reg = PM8607_##ureg,				\
+		.apply_bit = (ubit),					\
 		.enable_reg = PM8607_##ereg,				\
 		.enable_mask = 1 << (ebit),				\
 	},								\
-	.update_reg	= PM8607_##ureg,				\
-	.update_bit	= (ubit),					\
 	.slope_double	= (0),						\
 	.vol_table	= (unsigned int *)&vreg##_table,		\
 	.vol_suspend	= (unsigned int *)&vreg##_suspend_table,	\

@@ -343,9 +318,15 @@ static struct regulator_ops pm8606_preg_ops = {
 }
 
 static struct pm8607_regulator_info pm8607_regulator_info[] = {
-	PM8607_DVC(BUCK1, GO, 0, SUPPLIES_EN11, 0),
-	PM8607_DVC(BUCK2, GO, 1, SUPPLIES_EN11, 1),
-	PM8607_DVC(BUCK3, GO, 2, SUPPLIES_EN11, 2),
+	PM8607_DVC(BUCK1, GO, BIT(0), SUPPLIES_EN11, 0),
+	/*
+	 * BUCK2 is designed to control voltage of CP domain.
+	 * Linux only runs in AP side, so we don't need to control BUCK2 in AP
+	 * side. Thus set the apply bit to be 0 instead of BIT(1) to avoid
+	 * updating apply_bit for BUCK2.
+	 */
+	PM8607_DVC(BUCK2, GO, 0, SUPPLIES_EN11, 1),
+	PM8607_DVC(BUCK3, GO, BIT(2), SUPPLIES_EN11, 2),
 
 	PM8607_LDO(1,         LDO1, 0, SUPPLIES_EN11, 3),
 	PM8607_LDO(2,         LDO2, 0, SUPPLIES_EN11, 4),

Signature · 1 line

1.7.9.5

Haojian Zhuang

2012-12-21 16:07 UTC #2 lore raw

Re: [PATCH 2/4] regulator: 88pm8607: Use apply_[reg|bit] with regmap based voltage_sel operations

Quoted 76 lines · Axel Lin

Signed-off-by Axel Lin <[email protected]>

---
 drivers/regulator/88pm8607.c |   43 ++++++++++++------------------------------
 1 file changed, 12 insertions(+), 31 deletions(-)

drivers/regulator/88pm8607.c +3 ~~-29~~

index 2b55711..9ebdf8c 100644
@@ -30,8 +30,6 @@ struct pm8607_regulator_info {
        unsigned int    *vol_table;
        unsigned int    *vol_suspend;
 
-       int     update_reg;
-       int     update_bit;
        int     slope_double;
 };

@@ -222,29 +220,6 @@ static int pm8607_list_voltage(struct regulator_dev *rdev, unsigned index)
        return ret;
 }
 
-static int pm8607_set_voltage_sel(struct regulator_dev *rdev, unsigned selector)
-{
-       struct pm8607_regulator_info *info = rdev_get_drvdata(rdev);
-       uint8_t val;
-       int ret;
-
-       val = (uint8_t)(selector << (ffs(rdev->desc->vsel_mask) - 1));
-
-       ret = pm860x_set_bits(info->i2c, rdev->desc->vsel_reg,
-                             rdev->desc->vsel_mask, val);
-       if (ret)
-               return ret;
-       switch (info->desc.id) {
-       case PM8607_ID_BUCK1:
-       case PM8607_ID_BUCK3:
-               ret = pm860x_set_bits(info->i2c, info->update_reg,
-                                     1 << info->update_bit,
-                                     1 << info->update_bit);
-               break;
-       }
-       return ret;
-}
-
 static int pm8606_preg_enable(struct regulator_dev *rdev)
 {
        struct pm8607_regulator_info *info = rdev_get_drvdata(rdev);

@@ -276,7 +251,7 @@ static int pm8606_preg_is_enabled(struct regulator_dev *rdev)
 
 static struct regulator_ops pm8607_regulator_ops = {
        .list_voltage   = pm8607_list_voltage,
-       .set_voltage_sel = pm8607_set_voltage_sel,
+       .set_voltage_sel = regulator_set_voltage_sel_regmap,
        .get_voltage_sel = regulator_get_voltage_sel_regmap,
        .enable = regulator_enable_regmap,
        .disable = regulator_disable_regmap,

@@ -313,11 +288,11 @@ static struct regulator_ops pm8606_preg_ops = {
                .n_voltages = ARRAY_SIZE(vreg##_table),                 \
                .vsel_reg = PM8607_##vreg,                              \
                .vsel_mask = ARRAY_SIZE(vreg##_table) - 1,              \
+               .apply_reg = PM8607_##ureg,                             \
+               .apply_bit = (ubit),                                    \
                .enable_reg = PM8607_##ereg,                            \
                .enable_mask = 1 << (ebit),                             \
        },                                                              \
-       .update_reg     = PM8607_##ureg,                                \
-       .update_bit     = (ubit),                                       \
        .slope_double   = (0),                                          \
        .vol_table      = (unsigned int *)&vreg##_table,                \
        .vol_suspend    = (unsigned int *)&vreg##_suspend_table,        \

@@ -343,9 +318,15 @@ static struct regulator_ops pm8606_preg_ops = {
 }
 
 static struct pm8607_regulator_info pm8607_regulator_info[] = {
-       PM8607_DVC(BUCK1, GO, 0, SUPPLIES_EN11, 0),

drivers/regulator/88pm8607.c static struct regulator_ops pm8606_preg_ops = { · line 328

Quoted 10 lines

drivers/regulator/88pm8607.c +8 -2

-       PM8607_DVC(BUCK2, GO, 1, SUPPLIES_EN11, 1),
-       PM8607_DVC(BUCK3, GO, 2, SUPPLIES_EN11, 2),
+       PM8607_DVC(BUCK1, GO, BIT(0), SUPPLIES_EN11, 0),
+       /*
+        * BUCK2 is designed to control voltage of CP domain.
+        * Linux only runs in AP side, so we don't need to control BUCK2 in AP
+        * side. Thus set the apply bit to be 0 instead of BIT(1) to avoid
+        * updating apply_bit for BUCK2.
+        */
+       PM8607_DVC(BUCK2, GO, 0, SUPPLIES_EN11, 1),

Since update bit is already defined in GO register of PM8607, it seems better to
define it as BIT(1) in this register. Otherwise, the comment is so strange.

I'm sorry on this since I didn't look at your patch carefully in the first round. Please
help to change it as

      PM8607_DVC(BUCK2, GO, BIT(1), SUPPLIES_EN11, 1).

Since I messed it with the implementation of set_voltage_sel() api.

For others, Acked-by: Haojian Zhuang <[email protected]>

Quoted 9 lines

drivers/regulator/88pm8607.c +1 -1

+       PM8607_DVC(BUCK3, GO, BIT(2), SUPPLIES_EN11, 2),
 
        PM8607_LDO(1,         LDO1, 0, SUPPLIES_EN11, 3),
        PM8607_LDO(2,         LDO2, 0, SUPPLIES_EN11, 4),
--

1.7.9.5

Axel Lin

author

2012-12-18 01:32 UTC #3 lore raw

PATCH 3/4 regulator: da9052: Use apply_[reg|bit] with regmap based voltage_sel operations

In reply to Axel Lin

Signed-off-by Axel Lin <[email protected]>

---
 drivers/regulator/da9052-regulator.c |   41 +++++-----------------------------
 1 file changed, 6 insertions(+), 35 deletions(-)

drivers/regulator/da9052-regulator.c +6 ~~-35~~

index d096309..c6d8651 100644
@@ -70,7 +70,6 @@ struct da9052_regulator_info {
 	int step_uV;
 	int min_uV;
 	int max_uV;
-	unsigned char activate_bit;
 };
 
 struct da9052_regulator {

@@ -210,36 +209,6 @@ static int da9052_map_voltage(struct regulator_dev *rdev,
 	return sel;
 }
 
-static int da9052_regulator_set_voltage_sel(struct regulator_dev *rdev,
-					    unsigned int selector)
-{
-	struct da9052_regulator *regulator = rdev_get_drvdata(rdev);
-	struct da9052_regulator_info *info = regulator->info;
-	int id = rdev_get_id(rdev);
-	int ret;
-
-	ret = da9052_reg_update(regulator->da9052, rdev->desc->vsel_reg,
-				rdev->desc->vsel_mask, selector);
-	if (ret < 0)
-		return ret;
-
-	/* Some LDOs and DCDCs are DVC controlled which requires enabling of
-	 * the activate bit to implment the changes on the output.
-	 */
-	switch (id) {
-	case DA9052_ID_BUCK1:
-	case DA9052_ID_BUCK2:
-	case DA9052_ID_BUCK3:
-	case DA9052_ID_LDO2:
-	case DA9052_ID_LDO3:
-		ret = da9052_reg_update(regulator->da9052, DA9052_SUPPLY_REG,
-					info->activate_bit, info->activate_bit);
-		break;
-	}
-
-	return ret;
-}
-
 static struct regulator_ops da9052_dcdc_ops = {
 	.get_current_limit = da9052_dcdc_get_current_limit,
 	.set_current_limit = da9052_dcdc_set_current_limit,

@@ -247,7 +216,7 @@ static struct regulator_ops da9052_dcdc_ops = {
 	.list_voltage = da9052_list_voltage,
 	.map_voltage = da9052_map_voltage,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
-	.set_voltage_sel = da9052_regulator_set_voltage_sel,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,

@@ -257,7 +226,7 @@ static struct regulator_ops da9052_ldo_ops = {
 	.list_voltage = da9052_list_voltage,
 	.map_voltage = da9052_map_voltage,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
-	.set_voltage_sel = da9052_regulator_set_voltage_sel,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,

@@ -274,13 +243,14 @@ static struct regulator_ops da9052_ldo_ops = {
 		.owner = THIS_MODULE,\
 		.vsel_reg = DA9052_BUCKCORE_REG + DA9052_ID_##_id, \
 		.vsel_mask = (1 << (sbits)) - 1,\
+		.apply_reg = DA9052_SUPPLY_REG, \
+		.apply_bit = (abits), \
 		.enable_reg = DA9052_BUCKCORE_REG + DA9052_ID_##_id, \
 		.enable_mask = 1 << (ebits),\
 	},\
 	.min_uV = (min) * 1000,\
 	.max_uV = (max) * 1000,\
 	.step_uV = (step) * 1000,\
-	.activate_bit = (abits),\
 }
 
 #define DA9052_DCDC(_id, step, min, max, sbits, ebits, abits) \

@@ -294,13 +264,14 @@ static struct regulator_ops da9052_ldo_ops = {
 		.owner = THIS_MODULE,\
 		.vsel_reg = DA9052_BUCKCORE_REG + DA9052_ID_##_id, \
 		.vsel_mask = (1 << (sbits)) - 1,\
+		.apply_reg = DA9052_SUPPLY_REG, \
+		.apply_bit = (abits), \
 		.enable_reg = DA9052_BUCKCORE_REG + DA9052_ID_##_id, \
 		.enable_mask = 1 << (ebits),\
 	},\
 	.min_uV = (min) * 1000,\
 	.max_uV = (max) * 1000,\
 	.step_uV = (step) * 1000,\
-	.activate_bit = (abits),\
 }
 
 static struct da9052_regulator_info da9052_regulator_info[] = {

Signature · 1 line

1.7.9.5

Michael Grzeschik

2013-10-02 10:15 UTC #4 lore raw

Re: [PATCH 3/4] regulator: da9052: Use apply_[reg|bit] with regmap based voltage_sel operations

Hi Axel, Mark,

we should not play to much with generous functions for this pmics. With this patch we loose the fix_io code path from da9052_reg_update.

include/linux/mfd/da9052.h
...

        if (da9052->fix_io) {
                ret = da9052->fix_io(da9052, reg);
                if (ret < 0)
                        return ret;

...

This fix_io used in drivers/mfd/da9052-i2c.c

/*
* There is an issue with DA9052 and DA9053_AA/BA/BB PMIC where the PMIC
* gets lockup up or fails to respond following a system reset.
* This fix is to follow any read or write with a dummy read to a safe
* register.
*/
static int da9052_i2c_fix(struct da9052 *da9052, unsigned char reg)

{
        int val;

        switch (da9052->chip_id) {
        case DA9052:
        case DA9053_AA:
        case DA9053_BA:
        case DA9053_BB:
                /* A dummy read to a safe register address. */
        if (!i2c_safe_reg(reg))
                        return regmap_read(da9052->regmap,
                                           DA9052_PARK_REGISTER,
                                           &val);
                break;
        default:
                /*
                 * For other chips parking of I2C register
                 * to a safe place is not required.
                 */
                break;
        }

        return 0;
}

I suggest that Mark queue the revert of this patch.

Thanks,
Michael

Quoted 106 lines · Axel Lin

Signed-off-by Axel Lin <[email protected]>

---
 drivers/regulator/da9052-regulator.c |   41 +++++-----------------------------
 1 file changed, 6 insertions(+), 35 deletions(-)

drivers/regulator/da9052-regulator.c +6 ~~-35~~

index d096309..c6d8651 100644
@@ -70,7 +70,6 @@ struct da9052_regulator_info {
 	int step_uV;
 	int min_uV;
 	int max_uV;
-	unsigned char activate_bit;
 };
 
 struct da9052_regulator {

@@ -210,36 +209,6 @@ static int da9052_map_voltage(struct regulator_dev *rdev,
 	return sel;
 }
 
-static int da9052_regulator_set_voltage_sel(struct regulator_dev *rdev,
-					    unsigned int selector)
-{
-	struct da9052_regulator *regulator = rdev_get_drvdata(rdev);
-	struct da9052_regulator_info *info = regulator->info;
-	int id = rdev_get_id(rdev);
-	int ret;
-
-	ret = da9052_reg_update(regulator->da9052, rdev->desc->vsel_reg,
-				rdev->desc->vsel_mask, selector);
-	if (ret < 0)
-		return ret;
-
-	/* Some LDOs and DCDCs are DVC controlled which requires enabling of
-	 * the activate bit to implment the changes on the output.
-	 */
-	switch (id) {
-	case DA9052_ID_BUCK1:
-	case DA9052_ID_BUCK2:
-	case DA9052_ID_BUCK3:
-	case DA9052_ID_LDO2:
-	case DA9052_ID_LDO3:
-		ret = da9052_reg_update(regulator->da9052, DA9052_SUPPLY_REG,
-					info->activate_bit, info->activate_bit);
-		break;
-	}
-
-	return ret;
-}
-
 static struct regulator_ops da9052_dcdc_ops = {
 	.get_current_limit = da9052_dcdc_get_current_limit,
 	.set_current_limit = da9052_dcdc_set_current_limit,

@@ -247,7 +216,7 @@ static struct regulator_ops da9052_dcdc_ops = {
 	.list_voltage = da9052_list_voltage,
 	.map_voltage = da9052_map_voltage,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
-	.set_voltage_sel = da9052_regulator_set_voltage_sel,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,

@@ -257,7 +226,7 @@ static struct regulator_ops da9052_ldo_ops = {
 	.list_voltage = da9052_list_voltage,
 	.map_voltage = da9052_map_voltage,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
-	.set_voltage_sel = da9052_regulator_set_voltage_sel,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,

@@ -274,13 +243,14 @@ static struct regulator_ops da9052_ldo_ops = {
 		.owner = THIS_MODULE,\
 		.vsel_reg = DA9052_BUCKCORE_REG + DA9052_ID_##_id, \
 		.vsel_mask = (1 << (sbits)) - 1,\
+		.apply_reg = DA9052_SUPPLY_REG, \
+		.apply_bit = (abits), \
 		.enable_reg = DA9052_BUCKCORE_REG + DA9052_ID_##_id, \
 		.enable_mask = 1 << (ebits),\
 	},\
 	.min_uV = (min) * 1000,\
 	.max_uV = (max) * 1000,\
 	.step_uV = (step) * 1000,\
-	.activate_bit = (abits),\
 }
 
 #define DA9052_DCDC(_id, step, min, max, sbits, ebits, abits) \

@@ -294,13 +264,14 @@ static struct regulator_ops da9052_ldo_ops = {
 		.owner = THIS_MODULE,\
 		.vsel_reg = DA9052_BUCKCORE_REG + DA9052_ID_##_id, \
 		.vsel_mask = (1 << (sbits)) - 1,\
+		.apply_reg = DA9052_SUPPLY_REG, \
+		.apply_bit = (abits), \
 		.enable_reg = DA9052_BUCKCORE_REG + DA9052_ID_##_id, \
 		.enable_mask = 1 << (ebits),\
 	},\
 	.min_uV = (min) * 1000,\
 	.max_uV = (max) * 1000,\
 	.step_uV = (step) * 1000,\
-	.activate_bit = (abits),\
 }
 
 static struct da9052_regulator_info da9052_regulator_info[] = {

Signature · 1 line

1.7.9.5

Signature · 4 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Mark Brown

maintainer

2013-10-02 11:28 UTC #5 lore raw

Re: [PATCH 3/4] regulator: da9052: Use apply_[reg|bit] with regmap based voltage_sel operations

Quoted 4 lines · Michael Grzeschik

we should not play to much with generous functions for this pmics. With this patch we loose the fix_io code path from da9052_reg_update.
I suggest that Mark queue the revert of this patch.

This seems far too obscure to be robust - if we need to bodge all I2C I/O (which is what that looks like) then that should be being done in the regmap for the device rather than by having stuff open coded in the function drivers like this.

Axel Lin

author

2012-12-18 01:57 UTC #6 lore raw

PATCH 4/4 regulator: tps6586x: Use apply_[reg|bit] with regmap based voltage_sel operations

In reply to Axel Lin

Signed-off-by Axel Lin <[email protected]>

---
 drivers/regulator/tps6586x-regulator.c |   54 ++++++--------------------------
 1 file changed, 10 insertions(+), 44 deletions(-)

drivers/regulator/tps6586x-regulator.c +10 ~~-44~~

index f86da67..e68382d 100644
@@ -61,10 +61,6 @@ struct tps6586x_regulator {
 
 	int enable_bit[2];
 	int enable_reg[2];
-
-	/* for DVM regulators */
-	int go_reg;
-	int go_bit;
 };
 
 static inline struct device *to_tps6586x_dev(struct regulator_dev *rdev)

@@ -72,37 +68,10 @@ static inline struct device *to_tps6586x_dev(struct regulator_dev *rdev)
 	return rdev_get_dev(rdev)->parent;
 }
 
-static int tps6586x_set_voltage_sel(struct regulator_dev *rdev,
-				    unsigned selector)
-{
-	struct tps6586x_regulator *ri = rdev_get_drvdata(rdev);
-	struct device *parent = to_tps6586x_dev(rdev);
-	int ret, val, rid = rdev_get_id(rdev);
-	uint8_t mask;
-
-	val = selector << (ffs(rdev->desc->vsel_mask) - 1);
-	mask = rdev->desc->vsel_mask;
-
-	ret = tps6586x_update(parent, rdev->desc->vsel_reg, val, mask);
-	if (ret)
-		return ret;
-
-	/* Update go bit for DVM regulators */
-	switch (rid) {
-	case TPS6586X_ID_LDO_2:
-	case TPS6586X_ID_LDO_4:
-	case TPS6586X_ID_SM_0:
-	case TPS6586X_ID_SM_1:
-		ret = tps6586x_set_bits(parent, ri->go_reg, 1 << ri->go_bit);
-		break;
-	}
-	return ret;
-}
-
 static struct regulator_ops tps6586x_regulator_ops = {
 	.list_voltage = regulator_list_voltage_table,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
-	.set_voltage_sel = tps6586x_set_voltage_sel,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 
 	.is_enabled = regulator_is_enabled_regmap,
 	.enable = regulator_enable_regmap,

@@ -142,7 +111,7 @@ static const unsigned int tps6586x_dvm_voltages[] = {
 };
 
 #define TPS6586X_REGULATOR(_id, _pin_name, vdata, vreg, shift, nbits,	\
-			   ereg0, ebit0, ereg1, ebit1)			\
+			   ereg0, ebit0, ereg1, ebit1, goreg, gobit)	\
 	.desc	= {							\
 		.supply_name = _pin_name,				\
 		.name	= "REG-" #_id,					\

@@ -156,29 +125,26 @@ static const unsigned int tps6586x_dvm_voltages[] = {
 		.enable_mask = 1 << (ebit0),				\
 		.vsel_reg = TPS6586X_##vreg,				\
 		.vsel_mask = ((1 << (nbits)) - 1) << (shift),		\
+		.apply_reg = (goreg),				\
+		.apply_bit = (gobit),				\
 	},								\
 	.enable_reg[0]	= TPS6586X_SUPPLY##ereg0,			\
 	.enable_bit[0]	= (ebit0),					\
 	.enable_reg[1]	= TPS6586X_SUPPLY##ereg1,			\
 	.enable_bit[1]	= (ebit1),
 
-#define TPS6586X_REGULATOR_DVM_GOREG(goreg, gobit)			\
-	.go_reg = TPS6586X_##goreg,					\
-	.go_bit = (gobit),
-
 #define TPS6586X_LDO(_id, _pname, vdata, vreg, shift, nbits,		\
 		     ereg0, ebit0, ereg1, ebit1)			\
 {									\
 	TPS6586X_REGULATOR(_id, _pname, vdata, vreg, shift, nbits,	\
-			   ereg0, ebit0, ereg1, ebit1)			\
+			   ereg0, ebit0, ereg1, ebit1, 0, 0)		\
 }
 
 #define TPS6586X_DVM(_id, _pname, vdata, vreg, shift, nbits,		\
 		     ereg0, ebit0, ereg1, ebit1, goreg, gobit)		\
 {									\
 	TPS6586X_REGULATOR(_id, _pname, vdata, vreg, shift, nbits,	\
-			   ereg0, ebit0, ereg1, ebit1)			\
-	TPS6586X_REGULATOR_DVM_GOREG(goreg, gobit)			\
+			   ereg0, ebit0, ereg1, ebit1, goreg, gobit)	\
 }
 
 #define TPS6586X_SYS_REGULATOR()					\

@@ -207,13 +173,13 @@ static struct tps6586x_regulator tps6586x_regulator[] = {
 	TPS6586X_LDO(SM_2, "vin-sm2", sm2, SUPPLYV2, 0, 5, ENC, 7, END, 7),
 
 	TPS6586X_DVM(LDO_2, "vinldo23", dvm, LDO2BV1, 0, 5, ENA, 3,
-					ENB, 3, VCC2, 6),
+					ENB, 3, TPS6586X_VCC2, BIT(6)),
 	TPS6586X_DVM(LDO_4, "vinldo4", ldo4, LDO4V1, 0, 5, ENC, 3,
-					END, 3, VCC1, 6),
+					END, 3, TPS6586X_VCC1, BIT(6)),
 	TPS6586X_DVM(SM_0, "vin-sm0", dvm, SM0V1, 0, 5, ENA, 1,
-					ENB, 1, VCC1, 2),
+					ENB, 1, TPS6586X_VCC1, BIT(2)),
 	TPS6586X_DVM(SM_1, "vin-sm1", dvm, SM1V1, 0, 5, ENA, 0,
-					ENB, 0, VCC1, 0),
+					ENB, 0, TPS6586X_VCC1, BIT(0)),
 };
 
 /*

Signature · 1 line

1.7.9.5

Haojian Zhuang

Acked-by on v1 1/4

2012-12-21 16:21 UTC #7 lore raw

In reply to Axel Lin

Quoted 6 lines · Axel Lin

Some DVM regulators needs to update apply_bit after setting vsel_reg to initiate voltage change on the output. This patch adds apply_reg and apply_bit to struct regulator_desc and update regulator_set_voltage_sel_regmap() to set apply_bit of apply_reg when apply_bit is set.

Quoted 6 lines

Signed-off-by Axel Lin <[email protected]>

---
 drivers/regulator/core.c         |   12 +++++++++++-
 include/linux/regulator/driver.h |    6 ++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

Acked-by Haojian Zhuang <[email protected]>

Mark Brown

2012-12-24 16:34 UTC #8 lore raw

In reply to Axel Lin

Quoted 2 lines · Axel Lin

Some DVM regulators needs to update apply_bit after setting vsel_reg to initiate voltage change on the output. This patch adds apply_reg and

Applied all except patch 2 (where you've done a new version, I'll wait for Haojian to ack though) - thanks!

Haojian Zhuang

2012-12-25 00:38 UTC #9 lore raw

Quoted 6 lines · Mark Brown

Quoted 2 lines · Axel Lin
Some DVM regulators needs to update apply_bit after setting vsel_reg to initiate voltage change on the output. This patch adds apply_reg and
Applied all except patch 2 (where you've done a new version, I'll wait for Haojian to ack though) - thanks!

I acked 2nd patch just now.

Best Regards
Haojian