[PATCH 1/2] regulator: palmas: configure enable time for LDOs

[PATCH 1/2] regulator: palmas: configure enable time for LDOs

[Laxman Dewangan]

As per datasheet (Referred TPS65913), the on-time for LDO is
500micro second. If LDO6 is in vibrator mode then the on-time
is 2000us.

Set the enable_time on regulator descriptor accordingly.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/regulator/palmas-regulator.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c
index cb65c6f..fba4faa 100644
--- a/drivers/regulator/palmas-regulator.c
+++ b/drivers/regulator/palmas-regulator.c
@@ -979,6 +979,7 @@ static int palmas_regulators_probe(struct platform_device *pdev)
 			pmic->desc[id].min_uV = 900000;
 			pmic->desc[id].uV_step = 50000;
 			pmic->desc[id].linear_min_sel = 1;
+			pmic->desc[id].enable_time = 500;
 			pmic->desc[id].vsel_reg =
 					PALMAS_BASE_TO_REG(PALMAS_LDO_BASE,
 						palmas_regs_info[id].vsel_addr);
@@ -997,6 +998,11 @@ static int palmas_regulators_probe(struct platform_device *pdev)
 				pmic->desc[id].min_uV = 450000;
 				pmic->desc[id].uV_step = 25000;
 			}
+
+			/* LOD6 in vibrator mode will have enable time 2000us */
+			if (pdata && pdata->ldo6_vibrator &&
+				(id == PALMAS_REG_LDO6))
+				pmic->desc[id].enable_time = 2000;
 		} else {
 			pmic->desc[id].n_voltages = 1;
 			pmic->desc[id].ops = &palmas_ops_extreg;
-- 
1.7.1.1

[PATCH 2/2] regulator: core: add support for configuring turn-on time through constraints

[Laxman Dewangan]

The Turn-ON time of the regulator depends on the regulator device's
electrical characteristics. Sometimes regulator turn-on time also
depends on the capacitive load on the given platform and it can be
more than the datasheet value.

The driver provides the enable-time as per datasheet.

Add support for configure the enable time through regulator constraints
so that regulator core can take this value for enable time for that
regulator.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 .../devicetree/bindings/regulator/regulator.txt    |    1 +
 drivers/regulator/core.c                           |    2 ++
 drivers/regulator/of_regulator.c                   |    6 ++++++
 include/linux/regulator/machine.h                  |    2 ++
 4 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index 2bd8f09..f04b165 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -14,6 +14,7 @@ Optional properties:
 - regulator-ramp-delay: ramp delay for regulator(in uV/uS)
   For hardwares which support disabling ramp rate, it should be explicitly
   intialised to zero (regulator-ramp-delay = <0>) for disabling ramp delay.
+- regulator-enable-time: Turn ON time for regulator(in uS)
 
 Deprecated properties:
 - regulator-compatible: If a regulator chip contains multiple
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index a01b8b3..8dea3a4 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1186,6 +1186,8 @@ overflow_err:
 
 static int _regulator_get_enable_time(struct regulator_dev *rdev)
 {
+	if (rdev->constraints->enable_time)
+		return rdev->constraints->enable_time;
 	if (!rdev->desc->ops->enable_time)
 		return rdev->desc->enable_time;
 	return rdev->desc->ops->enable_time(rdev);
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 7827384..3152c03 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -23,6 +23,8 @@ static void of_get_regulation_constraints(struct device_node *np,
 	const __be32 *min_uA, *max_uA, *ramp_delay;
 	struct property *prop;
 	struct regulation_constraints *constraints = &(*init_data)->constraints;
+	int ret;
+	u32 pval;
 
 	constraints->name = of_get_property(np, "regulator-name", NULL);
 
@@ -73,6 +75,10 @@ static void of_get_regulation_constraints(struct device_node *np,
 		else
 			constraints->ramp_disable = true;
 	}
+
+	ret = of_property_read_u32(np, "regulator-enable-time", &pval);
+	if (!ret)
+		constraints->enable_time = pval;
 }
 
 /**
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index 999b20c..a9cf1af 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -95,6 +95,7 @@ struct regulator_state {
  * @initial_state: Suspend state to set by default.
  * @initial_mode: Mode to set at startup.
  * @ramp_delay: Time to settle down after voltage change (unit: uV/us)
+ * @enable_time: Turn ON time of the rails (unit: us)
  */
 struct regulation_constraints {
 
@@ -129,6 +130,7 @@ struct regulation_constraints {
 	unsigned int initial_mode;
 
 	unsigned int ramp_delay;
+	unsigned int enable_time;
 
 	/* constraint flags */
 	unsigned always_on:1;	/* regulator never off when system is on */
-- 
1.7.1.1

Re: [PATCH 2/2] regulator: core: add support for configuring turn-on time through constraints

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 495 bytes --]

On Tue, Sep 10, 2013 at 04:48:08PM +0530, Laxman Dewangan wrote:

> +- regulator-enable-time: Turn ON time for regulator(in uS)

This is unclear - what is a "turn on time" and is this in addition to or
separate from the underlying enable time of the device?  It needs to be
clear that this is the time taken for the voltage to ramp to spec.  I'd
also expect it to be clear that this is not something that should need
to be provided normally.

Please ALSO avoid capitalising words FOR no reason.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] regulator: palmas: configure enable time for LDOs

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 223 bytes --]

On Tue, Sep 10, 2013 at 04:48:07PM +0530, Laxman Dewangan wrote:
> As per datasheet (Referred TPS65913), the on-time for LDO is
> 500micro second. If LDO6 is in vibrator mode then the on-time
> is 2000us.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] regulator: core: add support for configuring turn-on time through constraints

[Laxman Dewangan]

On Tuesday 10 September 2013 05:39 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Tue, Sep 10, 2013 at 04:48:08PM +0530, Laxman Dewangan wrote:
>
>> +- regulator-enable-time: Turn ON time for regulator(in uS)
> This is unclear - what is a "turn on time" and is this in addition to or
> separate from the underlying enable time of the device?  It needs to be
> clear that this is the time taken for the voltage to ramp to spec.  I'd
> also expect it to be clear that this is not something that should need
> to be provided normally.

Here my intention for turn-on time is the time from OFF state to ON 
state and the ON voltage settled on this time.
This time is separate from underlying enable time of the device i.e. 
will not get added.
We observe in some of platform that some of regulator is taking more 
time than time given as per datasheet.
Yes, completely agree that this is not the data provided normally, just 
we observe on scope that some of platform shows  more than the datasheet 
value and hence this patch.

The same rail on other design does not show the enable time more than 
datasheet and on that case, we will not provide this value.



> Please ALSO avoid capitalising words FOR no reason.
>
>
My bad, will take care.

Re: [PATCH 2/2] regulator: core: add support for configuring turn-on time through constraints

[Stephen Warren]

On 09/10/2013 05:18 AM, Laxman Dewangan wrote:
> The Turn-ON time of the regulator depends on the regulator device's
> electrical characteristics. Sometimes regulator turn-on time also
> depends on the capacitive load on the given platform and it can be
> more than the datasheet value.
> 
> The driver provides the enable-time as per datasheet.
> 
> Add support for configure the enable time through regulator constraints
> so that regulator core can take this value for enable time for that
> regulator.

> diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt

> +- regulator-enable-time: Turn ON time for regulator(in uS)

Bike-shedding slightly: This isn't really the time it takes to enable a
regulator, but the time the voltage takes to become stable, or settle.
Perhaps name the property regulator-settle-time/regulator-settle-delay?

Re: [PATCH 2/2] regulator: core: add support for configuring turn-on time through constraints

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

On Tue, Sep 10, 2013 at 06:44:40PM +0530, Laxman Dewangan wrote:

> Here my intention for turn-on time is the time from OFF state to ON
> state and the ON voltage settled on this time.
> This time is separate from underlying enable time of the device i.e.
> will not get added.
> We observe in some of platform that some of regulator is taking more
> time than time given as per datasheet.
> Yes, completely agree that this is not the data provided normally,
> just we observe on scope that some of platform shows  more than the
> datasheet value and hence this patch.

So, put this in the binding document then! :P  This was more a request
to improve the detail in the binding than anything else.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] regulator: core: add support for configuring turn-on time through constraints

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 537 bytes --]

On Tue, Sep 10, 2013 at 09:11:13AM -0600, Stephen Warren wrote:

> > +- regulator-enable-time: Turn ON time for regulator(in uS)

> Bike-shedding slightly: This isn't really the time it takes to enable a
> regulator, but the time the voltage takes to become stable, or settle.
> Perhaps name the property regulator-settle-time/regulator-settle-delay?

The normal term would be ramp delay.  It's not usually the time taken to
completely settle, it's usually quoted as the time taken to reach within
some proportion of the target voltage.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] regulator: core: add support for configuring turn-on time through constraints

[Stephen Warren]

On 09/10/2013 10:51 AM, Mark Brown wrote:
> On Tue, Sep 10, 2013 at 09:11:13AM -0600, Stephen Warren wrote:
> 
>>> +- regulator-enable-time: Turn ON time for regulator(in uS)
> 
>> Bike-shedding slightly: This isn't really the time it takes to
>> enable a regulator, but the time the voltage takes to become
>> stable, or settle. Perhaps name the property
>> regulator-settle-time/regulator-settle-delay?
> 
> The normal term would be ramp delay.  It's not usually the time
> taken to completely settle, it's usually quoted as the time taken
> to reach within some proportion of the target voltage.

I notice there's a regulator-ramp-delay property, already documented
right above this new property. Is this a conflicting usage of the same
term, or should that existing property just be used in this case too?

Re: [PATCH 2/2] regulator: core: add support for configuring turn-on time through constraints

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 412 bytes --]

On Tue, Sep 10, 2013 at 11:34:13AM -0600, Stephen Warren wrote:

> I notice there's a regulator-ramp-delay property, already documented
> right above this new property. Is this a conflicting usage of the same
> term, or should that existing property just be used in this case too?

That's for a ramp between two voltages rather than the on/off voltage,
though I had forgotten about it.  Hrm.  enable-ramp-delay?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] regulator: core: add support for configuring turn-on time through constraints

[Stephen Warren]

On 09/10/2013 11:38 AM, Mark Brown wrote:
> On Tue, Sep 10, 2013 at 11:34:13AM -0600, Stephen Warren wrote:
> 
>> I notice there's a regulator-ramp-delay property, already documented
>> right above this new property. Is this a conflicting usage of the same
>> term, or should that existing property just be used in this case too?
> 
> That's for a ramp between two voltages rather than the on/off voltage,
> though I had forgotten about it.  Hrm.  enable-ramp-delay?

Sounds reasonable. It's a little long but regulator-enable-ramp-delay
might be better since all the common regulator properties to date are
named regulator-xxx.

Re: [PATCH 2/2] regulator: core: add support for configuring turn-on time through constraints

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 287 bytes --]

On Tue, Sep 10, 2013 at 11:45:41AM -0600, Stephen Warren wrote:

> Sounds reasonable. It's a little long but regulator-enable-ramp-delay
> might be better since all the common regulator properties to date are
> named regulator-xxx.

Yes, I just didn't bother typing the whole thing out.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]