* [PATCH 2/4] regulator: 88pm8607: Use apply_[reg|bit] with regmap based voltage_sel operations
2012-12-18 1:30 [PATCH 1/4] regulator: core: Allow specify apply_[reg|bit] for regmap based voltage_sel operations Axel Lin
@ 2012-12-18 1:31 ` Axel Lin
2012-12-21 15:58 ` Haojian Zhuang
2012-12-18 1:32 ` [PATCH 3/4] regulator: da9052: " Axel Lin
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Axel Lin @ 2012-12-18 1:31 UTC (permalink / raw)
To: Mark Brown; +Cc: Haojian Zhuang, Liam Girdwood, linux-kernel
Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
drivers/regulator/88pm8607.c | 43 ++++++++++++------------------------------
1 file changed, 12 insertions(+), 31 deletions(-)
diff --git a/drivers/regulator/88pm8607.c b/drivers/regulator/88pm8607.c
index 2b55711..9ebdf8c 100644
--- a/drivers/regulator/88pm8607.c
+++ b/drivers/regulator/88pm8607.c
@@ -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),
--
1.7.9.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/4] regulator: 88pm8607: Use apply_[reg|bit] with regmap based voltage_sel operations
2012-12-18 1:31 ` [PATCH 2/4] regulator: 88pm8607: Use apply_[reg|bit] with " Axel Lin
@ 2012-12-21 15:58 ` Haojian Zhuang
0 siblings, 0 replies; 10+ messages in thread
From: Haojian Zhuang @ 2012-12-21 15:58 UTC (permalink / raw)
To: Axel Lin; +Cc: Mark Brown, Liam Girdwood, linux-kernel@vger.kernel.org
On Tue, Dec 18, 2012 at 9:31 AM, Axel Lin <axel.lin@ingics.com> wrote:
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
> drivers/regulator/88pm8607.c | 43 ++++++++++++------------------------------
> 1 file changed, 12 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/regulator/88pm8607.c b/drivers/regulator/88pm8607.c
> index 2b55711..9ebdf8c 100644
> --- a/drivers/regulator/88pm8607.c
> +++ b/drivers/regulator/88pm8607.c
> @@ -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),
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 <haojian.zhuang@gmail.com>
> + 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
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] regulator: da9052: Use apply_[reg|bit] with regmap based voltage_sel operations
2012-12-18 1:30 [PATCH 1/4] regulator: core: Allow specify apply_[reg|bit] for regmap based voltage_sel operations Axel Lin
2012-12-18 1:31 ` [PATCH 2/4] regulator: 88pm8607: Use apply_[reg|bit] with " Axel Lin
@ 2012-12-18 1:32 ` Axel Lin
2013-10-02 10:15 ` Michael Grzeschik
2012-12-18 1:34 ` [PATCH 4/4] regulator: tps6586x: " Axel Lin
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Axel Lin @ 2012-12-18 1:32 UTC (permalink / raw)
To: Mark Brown; +Cc: David Dajun Chen, Ashish Jangam, Liam Girdwood, linux-kernel
Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
drivers/regulator/da9052-regulator.c | 41 +++++-----------------------------
1 file changed, 6 insertions(+), 35 deletions(-)
diff --git a/drivers/regulator/da9052-regulator.c b/drivers/regulator/da9052-regulator.c
index d096309..c6d8651 100644
--- a/drivers/regulator/da9052-regulator.c
+++ b/drivers/regulator/da9052-regulator.c
@@ -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[] = {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/4] regulator: da9052: Use apply_[reg|bit] with regmap based voltage_sel operations
2012-12-18 1:32 ` [PATCH 3/4] regulator: da9052: " Axel Lin
@ 2013-10-02 10:15 ` Michael Grzeschik
2013-10-02 11:28 ` Mark Brown
0 siblings, 1 reply; 10+ messages in thread
From: Michael Grzeschik @ 2013-10-02 10:15 UTC (permalink / raw)
To: Axel Lin
Cc: Mark Brown, David Dajun Chen, Ashish Jangam, Liam Girdwood,
linux-kernel, kernel
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
On Tue, Dec 18, 2012 at 09:32:52AM +0800, Axel Lin wrote:
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
> drivers/regulator/da9052-regulator.c | 41 +++++-----------------------------
> 1 file changed, 6 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/regulator/da9052-regulator.c b/drivers/regulator/da9052-regulator.c
> index d096309..c6d8651 100644
> --- a/drivers/regulator/da9052-regulator.c
> +++ b/drivers/regulator/da9052-regulator.c
> @@ -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[] = {
> --
> 1.7.9.5
--
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 |
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 3/4] regulator: da9052: Use apply_[reg|bit] with regmap based voltage_sel operations
2013-10-02 10:15 ` Michael Grzeschik
@ 2013-10-02 11:28 ` Mark Brown
0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2013-10-02 11:28 UTC (permalink / raw)
To: Michael Grzeschik
Cc: Axel Lin, David Dajun Chen, Ashish Jangam, Liam Girdwood,
linux-kernel, kernel
[-- Attachment #1: Type: text/plain, Size: 505 bytes --]
On Wed, Oct 02, 2013 at 12:15:00PM +0200, Michael Grzeschik wrote:
> 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.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] regulator: tps6586x: Use apply_[reg|bit] with regmap based voltage_sel operations
2012-12-18 1:30 [PATCH 1/4] regulator: core: Allow specify apply_[reg|bit] for regmap based voltage_sel operations Axel Lin
2012-12-18 1:31 ` [PATCH 2/4] regulator: 88pm8607: Use apply_[reg|bit] with " Axel Lin
2012-12-18 1:32 ` [PATCH 3/4] regulator: da9052: " Axel Lin
@ 2012-12-18 1:34 ` Axel Lin
2012-12-21 15:53 ` [PATCH 1/4] regulator: core: Allow specify apply_[reg|bit] for " Haojian Zhuang
2012-12-24 16:34 ` Mark Brown
4 siblings, 0 replies; 10+ messages in thread
From: Axel Lin @ 2012-12-18 1:34 UTC (permalink / raw)
To: Mark Brown; +Cc: Mike Rapoport, Liam Girdwood, linux-kernel
Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
drivers/regulator/tps6586x-regulator.c | 54 ++++++--------------------------
1 file changed, 10 insertions(+), 44 deletions(-)
diff --git a/drivers/regulator/tps6586x-regulator.c b/drivers/regulator/tps6586x-regulator.c
index f86da67..e68382d 100644
--- a/drivers/regulator/tps6586x-regulator.c
+++ b/drivers/regulator/tps6586x-regulator.c
@@ -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)),
};
/*
--
1.7.9.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 1/4] regulator: core: Allow specify apply_[reg|bit] for regmap based voltage_sel operations
2012-12-18 1:30 [PATCH 1/4] regulator: core: Allow specify apply_[reg|bit] for regmap based voltage_sel operations Axel Lin
` (2 preceding siblings ...)
2012-12-18 1:34 ` [PATCH 4/4] regulator: tps6586x: " Axel Lin
@ 2012-12-21 15:53 ` Haojian Zhuang
2012-12-24 16:34 ` Mark Brown
4 siblings, 0 replies; 10+ messages in thread
From: Haojian Zhuang @ 2012-12-21 15:53 UTC (permalink / raw)
To: Axel Lin
Cc: Mark Brown, David Dajun Chen, Ashish Jangam, Mike Rapoport,
Liam Girdwood, linux-kernel@vger.kernel.org
On Tue, Dec 18, 2012 at 9:30 AM, Axel Lin <axel.lin@ingics.com> wrote:
> 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 <axel.lin@ingics.com>
> ---
> drivers/regulator/core.c | 12 +++++++++++-
> include/linux/regulator/driver.h | 6 ++++++
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/4] regulator: core: Allow specify apply_[reg|bit] for regmap based voltage_sel operations
2012-12-18 1:30 [PATCH 1/4] regulator: core: Allow specify apply_[reg|bit] for regmap based voltage_sel operations Axel Lin
` (3 preceding siblings ...)
2012-12-21 15:53 ` [PATCH 1/4] regulator: core: Allow specify apply_[reg|bit] for " Haojian Zhuang
@ 2012-12-24 16:34 ` Mark Brown
2012-12-25 0:38 ` Haojian Zhuang
4 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2012-12-24 16:34 UTC (permalink / raw)
To: Axel Lin
Cc: Haojian Zhuang, David Dajun Chen, Ashish Jangam, Mike Rapoport,
Liam Girdwood, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 313 bytes --]
On Tue, Dec 18, 2012 at 09:30:10AM +0800, Axel Lin wrote:
> 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!
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/4] regulator: core: Allow specify apply_[reg|bit] for regmap based voltage_sel operations
2012-12-24 16:34 ` Mark Brown
@ 2012-12-25 0:38 ` Haojian Zhuang
0 siblings, 0 replies; 10+ messages in thread
From: Haojian Zhuang @ 2012-12-25 0:38 UTC (permalink / raw)
To: Mark Brown
Cc: Axel Lin, David Dajun Chen, Ashish Jangam, Mike Rapoport,
Liam Girdwood, linux-kernel@vger.kernel.org
On Tue, Dec 25, 2012 at 12:34 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Dec 18, 2012 at 09:30:10AM +0800, Axel Lin wrote:
>> 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
^ permalink raw reply [flat|nested] 10+ messages in thread