2012-06-19 07:54:12

by Yadwinder Singh Brar

[permalink] [raw]
Subject: [PATCH 1/2] regulator: max77686: Convert driver to use regulator_set_voltage_time_sel.

This patch converts the driver to use regulator_set_voltage_time_sel() as
.set_voltage_time_sel() callback. It also sets ramp_delay as 100000 uV/us for
LDOs & normal BUCKs and 27500 uV/us(default/reset value) for BUCKs[2/3/4] in
regulator_desc[].

Signed-off-by: Yadwinder Singh Brar <[email protected]>
---
drivers/regulator/max77686.c | 42 +++++++++---------------------------------
1 files changed, 9 insertions(+), 33 deletions(-)

diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
index 2dd4ac9..a29eee3 100644
--- a/drivers/regulator/max77686.c
+++ b/drivers/regulator/max77686.c
@@ -40,6 +40,8 @@
#define MAX77686_LDO_LOW_UVSTEP 25000
#define MAX77686_BUCK_MINUV 750000
#define MAX77686_BUCK_UVSTEP 50000
+#define MAX77686_RAMP_DELAY 100000 /* uV/us */
+#define MAX77686_DVS_RAMP_DELAY 27500 /* uV/us */
#define MAX77686_DVS_MINUV 600000
#define MAX77686_DVS_UVSTEP 12500

@@ -66,28 +68,8 @@ struct max77686_data {
struct device *dev;
struct max77686_dev *iodev;
struct regulator_dev **rdev;
- int ramp_delay; /* in mV/us */
};

-static int max77686_set_dvs_voltage_time_sel(struct regulator_dev *rdev,
- unsigned int old_selector, unsigned int new_selector)
-{
- struct max77686_data *max77686 = rdev_get_drvdata(rdev);
- int ramp_rate[] = {13, 27, 55, 100};
-
- return DIV_ROUND_UP(rdev->desc->uV_step *
- abs(new_selector - old_selector),
- ramp_rate[max77686->ramp_delay] * 1000);
-}
-
-static int max77686_set_voltage_time_sel(struct regulator_dev *rdev,
- unsigned int old_selector, unsigned int new_selector)
-{
- /* Unconditionally 100 mV/us */
- return DIV_ROUND_UP(rdev->desc->uV_step *
- abs(new_selector - old_selector), 100 * 1000);
-}
-
static struct regulator_ops max77686_ops = {
.list_voltage = regulator_list_voltage_linear,
.map_voltage = regulator_map_voltage_linear,
@@ -96,7 +78,7 @@ static struct regulator_ops max77686_ops = {
.disable = regulator_disable_regmap,
.get_voltage_sel = regulator_get_voltage_sel_regmap,
.set_voltage_sel = regulator_set_voltage_sel_regmap,
- .set_voltage_time_sel = max77686_set_voltage_time_sel,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
};

static struct regulator_ops max77686_buck_dvs_ops = {
@@ -107,7 +89,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
.disable = regulator_disable_regmap,
.get_voltage_sel = regulator_get_voltage_sel_regmap,
.set_voltage_sel = regulator_set_voltage_sel_regmap,
- .set_voltage_time_sel = max77686_set_dvs_voltage_time_sel,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
};

#define regulator_desc_ldo(num) { \
@@ -118,6 +100,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
.owner = THIS_MODULE, \
.min_uV = MAX77686_LDO_MINUV, \
.uV_step = MAX77686_LDO_UVSTEP, \
+ .ramp_delay = MAX77686_RAMP_DELAY, \
.n_voltages = MAX77686_VSEL_MASK + 1, \
.vsel_reg = MAX77686_REG_LDO1CTRL1 + num - 1, \
.vsel_mask = MAX77686_VSEL_MASK, \
@@ -133,6 +116,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
.owner = THIS_MODULE, \
.min_uV = MAX77686_LDO_LOW_MINUV, \
.uV_step = MAX77686_LDO_LOW_UVSTEP, \
+ .ramp_delay = MAX77686_RAMP_DELAY, \
.n_voltages = MAX77686_VSEL_MASK + 1, \
.vsel_reg = MAX77686_REG_LDO1CTRL1 + num - 1, \
.vsel_mask = MAX77686_VSEL_MASK, \
@@ -148,6 +132,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
.owner = THIS_MODULE, \
.min_uV = MAX77686_BUCK_MINUV, \
.uV_step = MAX77686_BUCK_UVSTEP, \
+ .ramp_delay = MAX77686_RAMP_DELAY, \
.n_voltages = MAX77686_VSEL_MASK + 1, \
.vsel_reg = MAX77686_REG_BUCK5OUT + (num - 5) * 2, \
.vsel_mask = MAX77686_VSEL_MASK, \
@@ -162,6 +147,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
.owner = THIS_MODULE, \
.min_uV = MAX77686_BUCK_MINUV, \
.uV_step = MAX77686_BUCK_UVSTEP, \
+ .ramp_delay = MAX77686_RAMP_DELAY, \
.n_voltages = MAX77686_VSEL_MASK + 1, \
.vsel_reg = MAX77686_REG_BUCK1OUT, \
.vsel_mask = MAX77686_VSEL_MASK, \
@@ -176,6 +162,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
.owner = THIS_MODULE, \
.min_uV = MAX77686_DVS_MINUV, \
.uV_step = MAX77686_DVS_UVSTEP, \
+ .ramp_delay = MAX77686_DVS_RAMP_DELAY, \
.n_voltages = MAX77686_DVS_VSEL_MASK + 1, \
.vsel_reg = MAX77686_REG_BUCK2DVS1 + (num - 2) * 10, \
.vsel_mask = MAX77686_DVS_VSEL_MASK, \
@@ -255,17 +242,6 @@ static __devinit int max77686_pmic_probe(struct platform_device *pdev)
max77686->iodev = iodev;
platform_set_drvdata(pdev, max77686);

- max77686->ramp_delay = RAMP_RATE_NO_CTRL; /* Set 0x3 for RAMP */
- regmap_update_bits(max77686->iodev->regmap,
- MAX77686_REG_BUCK2CTRL1, MAX77686_RAMP_RATE_MASK,
- max77686->ramp_delay << 6);
- regmap_update_bits(max77686->iodev->regmap,
- MAX77686_REG_BUCK3CTRL1, MAX77686_RAMP_RATE_MASK,
- max77686->ramp_delay << 6);
- regmap_update_bits(max77686->iodev->regmap,
- MAX77686_REG_BUCK4CTRL1, MAX77686_RAMP_RATE_MASK,
- max77686->ramp_delay << 6);
-
for (i = 0; i < MAX77686_REGULATORS; i++) {
config.dev = max77686->dev;
config.regmap = iodev->regmap;
--
1.7.0.4


2012-06-19 07:54:23

by Yadwinder Singh Brar

[permalink] [raw]
Subject: [PATCH 2/2] regulator: max77686: Implement .set_ramp_delay() callback.

This patch implements the .set_ramp_delay callback to set the ramp_delay on
hardware for BUCK2/3/4 if ramp_delay is set in regulator constraints.

This patch also do some cleaning work for unrequired members of
struct max77686_data.

Signed-off-by: Yadwinder Singh Brar <[email protected]>
---
drivers/regulator/max77686.c | 38 ++++++++++++++++++++++++++++++--------
1 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
index a29eee3..535eb1f 100644
--- a/drivers/regulator/max77686.c
+++ b/drivers/regulator/max77686.c
@@ -65,11 +65,35 @@ enum max77686_ramp_rate {
};

struct max77686_data {
- struct device *dev;
- struct max77686_dev *iodev;
struct regulator_dev **rdev;
};

+static int max77686_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
+{
+ unsigned int ramp_value = RAMP_RATE_NO_CTRL;
+
+ switch (ramp_delay) {
+ case 1 ... 13750:
+ ramp_value = RAMP_RATE_13P75MV;
+ break;
+ case 13751 ... 27500:
+ ramp_value = RAMP_RATE_27P5MV;
+ break;
+ case 27501 ... 55000:
+ ramp_value = RAMP_RATE_55MV;
+ break;
+ case 55001 ... 100000:
+ break;
+ default:
+ pr_warn("%s: ramp_delay: %d not supported, setting 100000\n",
+ rdev->desc->name, ramp_delay);
+ }
+
+ return regmap_update_bits(rdev->regmap, MAX77686_REG_BUCK2CTRL1 +
+ (rdev->desc->id - MAX77686_BUCK2) * 10,
+ MAX77686_RAMP_RATE_MASK, ramp_value << 6);
+}
+
static struct regulator_ops max77686_ops = {
.list_voltage = regulator_list_voltage_linear,
.map_voltage = regulator_map_voltage_linear,
@@ -90,6 +114,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
.get_voltage_sel = regulator_get_voltage_sel_regmap,
.set_voltage_sel = regulator_set_voltage_sel_regmap,
.set_voltage_time_sel = regulator_set_voltage_time_sel,
+ .set_ramp_delay = max77686_set_ramp_delay,
};

#define regulator_desc_ldo(num) { \
@@ -238,20 +263,17 @@ static __devinit int max77686_pmic_probe(struct platform_device *pdev)
return -ENOMEM;

rdev = max77686->rdev;
- max77686->dev = &pdev->dev;
- max77686->iodev = iodev;
+ config.dev = &pdev->dev;
+ config.regmap = iodev->regmap;
platform_set_drvdata(pdev, max77686);

for (i = 0; i < MAX77686_REGULATORS; i++) {
- config.dev = max77686->dev;
- config.regmap = iodev->regmap;
- config.driver_data = max77686;
config.init_data = pdata->regulators[i].initdata;

rdev[i] = regulator_register(&regulators[i], &config);
if (IS_ERR(rdev[i])) {
ret = PTR_ERR(rdev[i]);
- dev_err(max77686->dev,
+ dev_err(&pdev->dev,
"regulator init failed for %d\n", i);
rdev[i] = NULL;
goto err;
--
1.7.0.4

2012-06-19 08:19:13

by Axel Lin

[permalink] [raw]
Subject: Re: [PATCH 2/2] regulator: max77686: Implement .set_ramp_delay() callback.

> ?static struct regulator_ops max77686_ops = {
> ? ? ? ?.list_voltage ? ? ? ? ? = regulator_list_voltage_linear,
> ? ? ? ?.map_voltage ? ? ? ? ? ?= regulator_map_voltage_linear,
> @@ -90,6 +114,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
> ? ? ? ?.get_voltage_sel ? ? ? ?= regulator_get_voltage_sel_regmap,
> ? ? ? ?.set_voltage_sel ? ? ? ?= regulator_set_voltage_sel_regmap,
> ? ? ? ?.set_voltage_time_sel ? = regulator_set_voltage_time_sel,
> + ? ? ? .set_ramp_delay ? ? ? ? = max77686_set_ramp_delay,
> ?};

I think what you want here is to set .set_ramp_delay callback for
max77686_buck_dvs_ops
rather than max77686_ops.

2012-06-19 08:21:08

by Yadwinder Singh Brar

[permalink] [raw]
Subject: Re: [PATCH 2/2] regulator: max77686: Implement .set_ramp_delay() callback.

On Tue, Jun 19, 2012 at 1:48 PM, Axel Lin <[email protected]> wrote:
>> ?static struct regulator_ops max77686_ops = {
>> ? ? ? ?.list_voltage ? ? ? ? ? = regulator_list_voltage_linear,
>> ? ? ? ?.map_voltage ? ? ? ? ? ?= regulator_map_voltage_linear,
>> @@ -90,6 +114,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
>> ? ? ? ?.get_voltage_sel ? ? ? ?= regulator_get_voltage_sel_regmap,
>> ? ? ? ?.set_voltage_sel ? ? ? ?= regulator_set_voltage_sel_regmap,
>> ? ? ? ?.set_voltage_time_sel ? = regulator_set_voltage_time_sel,
>> + ? ? ? .set_ramp_delay ? ? ? ? = max77686_set_ramp_delay,
>> ?};
>
> I think what you want here is to set .set_ramp_delay callback for
> max77686_buck_dvs_ops
> rather than max77686_ops.

Yes, I will correct it.

Thanks,
Yadwinder.

2012-06-19 11:10:29

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/2] regulator: max77686: Convert driver to use regulator_set_voltage_time_sel.

On Tue, Jun 19, 2012 at 01:23:42PM +0530, Yadwinder Singh Brar wrote:
> This patch converts the driver to use regulator_set_voltage_time_sel() as
> .set_voltage_time_sel() callback. It also sets ramp_delay as 100000 uV/us for
> LDOs & normal BUCKs and 27500 uV/us(default/reset value) for BUCKs[2/3/4] in
> regulator_desc[].

Applied, thanks.


Attachments:
(No filename) (343.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-06-19 11:57:52

by Yadwinder Singh Brar

[permalink] [raw]
Subject: Re: [PATCH 2/2] regulator: max77686: Implement .set_ramp_delay() callback.

On Tue, Jun 19, 2012 at 1:48 PM, Axel Lin <[email protected]> wrote:
>> ?static struct regulator_ops max77686_ops = {
>> ? ? ? ?.list_voltage ? ? ? ? ? = regulator_list_voltage_linear,
>> ? ? ? ?.map_voltage ? ? ? ? ? ?= regulator_map_voltage_linear,
>> @@ -90,6 +114,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {

^^^^^^^^^^^^^^^^^^^^^^^^^
>> ? ? ? ?.get_voltage_sel ? ? ? ?= regulator_get_voltage_sel_regmap,
>> ? ? ? ?.set_voltage_sel ? ? ? ?= regulator_set_voltage_sel_regmap,
>> ? ? ? ?.set_voltage_time_sel ? = regulator_set_voltage_time_sel,
>> + ? ? ? .set_ramp_delay ? ? ? ? = max77686_set_ramp_delay,
>> ?};
>
> I think what you want here is to set .set_ramp_delay callback for
> max77686_buck_dvs_ops
> rather than max77686_ops.

Now I have seen into my code after applying this patch again, yes its
set for max77686_buck_dvs_ops.
I also missed to catch this in patch... :) ..
Anyways, thanks for review and I think, we don't need to revise this patch.

2012-06-19 12:20:59

by Axel Lin

[permalink] [raw]
Subject: Re: [PATCH 2/2] regulator: max77686: Implement .set_ramp_delay() callback.

2012/6/19 Yadwinder Singh Brar <[email protected]>:
> On Tue, Jun 19, 2012 at 1:48 PM, Axel Lin <[email protected]> wrote:
>>> ?static struct regulator_ops max77686_ops = {
>>> ? ? ? ?.list_voltage ? ? ? ? ? = regulator_list_voltage_linear,
>>> ? ? ? ?.map_voltage ? ? ? ? ? ?= regulator_map_voltage_linear,
>>> @@ -90,6 +114,7 @@ static struct regulator_ops max77686_buck_dvs_ops = {
>
> ? ?^^^^^^^^^^^^^^^^^^^^^^^^^
>>> ? ? ? ?.get_voltage_sel ? ? ? ?= regulator_get_voltage_sel_regmap,
>>> ? ? ? ?.set_voltage_sel ? ? ? ?= regulator_set_voltage_sel_regmap,
>>> ? ? ? ?.set_voltage_time_sel ? = regulator_set_voltage_time_sel,
>>> + ? ? ? .set_ramp_delay ? ? ? ? = max77686_set_ramp_delay,
>>> ?};
>>
>> I think what you want here is to set .set_ramp_delay callback for
>> max77686_buck_dvs_ops
>> rather than max77686_ops.
>
> Now I have seen into my code after applying this patch again, yes its
> set for ?max77686_buck_dvs_ops.
> I also missed to catch this in patch... :) ..
> Anyways, thanks for review and I think, we don't need to revise this patch.

oh, sorry, my bad.

Just try to apply your patch, I found

+ return regmap_update_bits(rdev->regmap, MAX77686_REG_BUCK2CTRL1 +
+ (rdev->desc->id - MAX77686_BUCK2) * 10,
+ MAX77686_RAMP_RATE_MASK, ramp_value << 6);

can be simplified to:

return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
ramp_value << 6);

How do you think?

Regards,
Axel

2012-06-19 12:24:13

by Axel Lin

[permalink] [raw]
Subject: Re: [PATCH 2/2] regulator: max77686: Implement .set_ramp_delay() callback.

> return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> ? ? ? ? ? ? ? ? ? ? ? ? ?ramp_value << 6);

Typo, should be:
return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
MAX77686_RAMP_RATE_MASK, ramp_value << 6);

>
> How do you think?
>
> Regards,
> Axel

2012-06-19 15:20:12

by Yadwinder Singh Brar

[permalink] [raw]
Subject: Re: [PATCH 2/2] regulator: max77686: Implement .set_ramp_delay() callback.

On Tue, Jun 19, 2012 at 5:53 PM, Axel Lin <[email protected]> wrote:
>> return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
>> ? ? ? ? ? ? ? ? ? ? ? ? ?ramp_value << 6);
>
> Typo, should be:
> ? ? ? ?return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?MAX77686_RAMP_RATE_MASK, ramp_value << 6);

Ok, I will do it.

Thanks.