2013-04-20 12:13:00

by Axel Lin

[permalink] [raw]
Subject: [RFC/RFT][PATCH] regulator: palmas: Convert to use regulator_set_voltage_time_sel()

Use regulator_set_voltage_time_sel() instead of open coded.

If rdev->constraints->ramp_delay is specified, the setting will be used in
regulator_set_voltage_time_sel(). And then pmic->ramp_delay[] is not used and
can be removed.

There is a different behavior change here:
regulator_set_voltage_time_sel() always returns
DIV_ROUND_UP(abs(new_volt - old_volt), ramp_delay);

palma_smps_set_voltage_smps_time_sel() actually returns
DIV_ROUND_UP(abs(new_volt - old_volt), modified_ramp_delay);
where the modified_ramp_delay is not exactly specified by
rdev->constraints->ramp_delay but a value from pmic->ramp_delay[id].

So palma_smps_set_voltage_smps_time_sel() may return a smaller delay than
regulator_set_voltage_time_sel() depend on rdev->constraints->ramp_delay value.

I think the delay in both version are *safe* for the operation.

Signed-off-by: Axel Lin <[email protected]>
---
drivers/regulator/palmas-regulator.c | 26 +-------------------------
include/linux/mfd/palmas.h | 1 -
2 files changed, 1 insertion(+), 26 deletions(-)

diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
index 9254f94..3d9437e 100644
--- a/drivers/regulator/palmas-regulator.c
+++ b/drivers/regulator/palmas-regulator.c
@@ -408,28 +408,6 @@ static int palmas_map_voltage_smps(struct regulator_dev *rdev,
return ret;
}

-static int palma_smps_set_voltage_smps_time_sel(struct regulator_dev *rdev,
- unsigned int old_selector, unsigned int new_selector)
-{
- struct palmas_pmic *pmic = rdev_get_drvdata(rdev);
- int id = rdev_get_id(rdev);
- int old_uv, new_uv;
- unsigned int ramp_delay = pmic->ramp_delay[id];
-
- if (!ramp_delay)
- return 0;
-
- old_uv = palmas_list_voltage_smps(rdev, old_selector);
- if (old_uv < 0)
- return old_uv;
-
- new_uv = palmas_list_voltage_smps(rdev, new_selector);
- if (new_uv < 0)
- return new_uv;
-
- return DIV_ROUND_UP(abs(old_uv - new_uv), ramp_delay);
-}
-
static int palmas_smps_set_ramp_delay(struct regulator_dev *rdev,
int ramp_delay)
{
@@ -454,7 +432,6 @@ static int palmas_smps_set_ramp_delay(struct regulator_dev *rdev,
return ret;
}

- pmic->ramp_delay[id] = palmas_smps_ramp_delay[reg];
return ret;
}

@@ -468,7 +445,7 @@ static struct regulator_ops palmas_ops_smps = {
.set_voltage_sel = regulator_set_voltage_sel_regmap,
.list_voltage = palmas_list_voltage_smps,
.map_voltage = palmas_map_voltage_smps,
- .set_voltage_time_sel = palma_smps_set_voltage_smps_time_sel,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
.set_ramp_delay = palmas_smps_set_ramp_delay,
};

@@ -846,7 +823,6 @@ static int palmas_regulators_probe(struct platform_device *pdev)
}
pmic->desc[id].ramp_delay =
palmas_smps_ramp_delay[reg & 0x3];
- pmic->ramp_delay[id] = pmic->desc[id].ramp_delay;
}

/* Initialise sleep/init values from platform data */
diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
index 8f21daf..c288118 100644
--- a/include/linux/mfd/palmas.h
+++ b/include/linux/mfd/palmas.h
@@ -337,7 +337,6 @@ struct palmas_pmic {
int smps457;

int range[PALMAS_REG_SMPS10];
- unsigned int ramp_delay[PALMAS_REG_SMPS10];
unsigned int current_reg_mode[PALMAS_REG_SMPS10];
};

--
1.7.10.4



2013-04-21 08:52:40

by Laxman Dewangan

[permalink] [raw]
Subject: Re: [RFC/RFT][PATCH] regulator: palmas: Convert to use regulator_set_voltage_time_sel()

On Saturday 20 April 2013 05:42 PM, Axel Lin wrote:
> Use regulator_set_voltage_time_sel() instead of open coded.
>
> If rdev->constraints->ramp_delay is specified, the setting will be used in
> regulator_set_voltage_time_sel(). And then pmic->ramp_delay[] is not used and
> can be removed.
>
> There is a different behavior change here:
> regulator_set_voltage_time_sel() always returns
> DIV_ROUND_UP(abs(new_volt - old_volt), ramp_delay);
>
> palma_smps_set_voltage_smps_time_sel() actually returns
> DIV_ROUND_UP(abs(new_volt - old_volt), modified_ramp_delay);
> where the modified_ramp_delay is not exactly specified by
> rdev->constraints->ramp_delay but a value from pmic->ramp_delay[id].
>
> So palma_smps_set_voltage_smps_time_sel() may return a smaller delay than
> regulator_set_voltage_time_sel() depend on rdev->constraints->ramp_delay value.
>
> I think the delay in both version are *safe* for the operation.
>
Nack,
This approach does not look good and the reason for which I did not use
this core api here was:
- The palma device support ramp of 10mV/us, 5mV/us and 2.5mV/us. So if
client pass the other than this value then register is programmed to
nearest high value.
In this case, the constraint->ramp_delay is not much accurate with
register level programming and need to update with actual register
programmed. Hence we need to have local equivalent to the register
programmed and use this new value instead of constraints->ramp_delay.

- Second reason is that TPS65913 ES1.0/ES2.0 has the hw errata on which
the output voltage become slow ramp near to final value. TI suggested to
use 1.5x as SWAR. So we need to use the local implementation to adjust this.
The changes for this errata is not there because we will need to read
the version register and Ian has posted the series which is not merged
yet. So waiting for his series to be merged for this errata implementation.