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

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

[Axel Lin]

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 <axel.lin@ingics.com>
---
 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

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

[Laxman Dewangan]

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.