Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1345879 > unrolled thread

[PATCH 1/2] regulator: DT: Add support to scale ramp delay based on platform behavior

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/2] regulator: DT: Add support to scale ramp delay based on platform behavior Laxman Dewangan <ldewangan@nvidia.com> - 2016-02-29 16:00 +0100
    [PATCH 2/2] regulator: Add support to scale ramp delay based on platform behavior Laxman Dewangan <ldewangan@nvidia.com> - 2016-02-29 16:00 +0100
    Re: [PATCH 1/2] regulator: DT: Add support to scale ramp delay based  on platform behavior Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-02-29 18:50 +0100
      Re: [PATCH 1/2] regulator: DT: Add support to scale ramp delay based  on platform behavior Mark Brown <broonie@kernel.org> - 2016-03-01 04:10 +0100
        Re: [PATCH 1/2] regulator: DT: Add support to scale ramp delay based  on platform behavior Laxman Dewangan <ldewangan@nvidia.com> - 2016-03-01 05:10 +0100
          Re: [PATCH 1/2] regulator: DT: Add support to scale ramp delay based  on platform behavior Mark Brown <broonie@kernel.org> - 2016-03-02 04:40 +0100
            Re: [PATCH 1/2] regulator: DT: Add support to scale ramp delay based  on platform behavior Laxman Dewangan <ldewangan@nvidia.com> - 2016-03-02 04:50 +0100
              Re: [PATCH 1/2] regulator: DT: Add support to scale ramp delay based  on platform behavior Mark Brown <broonie@kernel.org> - 2016-03-02 05:40 +0100
                Re: [PATCH 1/2] regulator: DT: Add support to scale ramp delay based  on platform behavior Laxman Dewangan <ldewangan@nvidia.com> - 2016-03-02 07:30 +0100

#1345879 — [PATCH 1/2] regulator: DT: Add support to scale ramp delay based on platform behavior

It is observed that voltage change in given rail affected by the load
and the capacitor in the rail. This may cause the slow ramp in voltage
against what PMIC has programmed.

The property regulator-ramp-delay provides the ramp delay configuration
for PMIC but actual voltage settling time may change based on platform.

Add new property "regulator-ramp-delay-scale" for platform specific scaling
in final ramp delay calculation. On this case, final wait delay time
for voltage change is calculated as the
   DIV_ROUND_UP(ramp_delay * ramp_delay_scale, 100)

The value is provided in the term of percentage i.e. 300% means it will
do the 3x delay of calculated value.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 Documentation/devicetree/bindings/regulator/regulator.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index 1d112fc..f2e8360 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -16,6 +16,13 @@ Optional properties:
 - regulator-ramp-delay: ramp delay for regulator(in uV/uS)
   For hardware which supports disabling ramp rate, it should be explicitly
   initialised to zero (regulator-ramp-delay = <0>) for disabling ramp delay.
+- regulator-ramp-delay-scale: Platform specific scaling in ramp delay to
+  provide extra guard time to settle down voltage after its change. This will
+  help to wait for extra time for voltage settling based on platform behavior.
+  This is provided in terms of percentage scaling like 300% means final delay
+  will become 3x of calculated ramp delay for voltage change.
+  The final delay is calculated as:
+	delay = DIV_ROUND_UP(ramp-delay * ramp-delay-scale, 100)
 - regulator-enable-ramp-delay: The time taken, in microseconds, for the supply
   rail to reach the target voltage, plus/minus whatever tolerance the board
   design requires. This property describes the total system ramp time
-- 
2.1.4

[toc] | [next] | [standalone]

#1345880 — [PATCH 2/2] regulator: Add support to scale ramp delay based on platform behavior

It is observed that voltage change in given rail affected by the load
and the capacitor in the rail. This may cause the slow ramp in voltage
against what PMIC has programmed.

The machine constraints ramp_delay provides the ramp delay configuration
for PMIC but actual voltage settling time may change based on platform.

Add ramp_delay_scale in the machine constraints for platform specific
scaling for final ramp delay calculation. On this case, final wait
delay time for voltage change is calculated as the
	DIV_ROUND_UP(ramp_delay * ramp_delay_scale, 100)

The value is provided in the term of percentage i.e. 300% means it will
do the 3x delay of calculated value.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/regulator/core.c          | 6 ++++++
 drivers/regulator/of_regulator.c  | 7 +++++++
 include/linux/regulator/machine.h | 7 +++++++
 3 files changed, 20 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 055f8c1..476fe6c 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2786,6 +2786,12 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 			delay = 0;
 		}
 
+		if (delay && rdev->constraints &&
+		    rdev->constraints->ramp_delay_scale) {
+			delay *= rdev->constraints->ramp_delay_scale;
+			delay = DIV_ROUND_UP(delay, 100);
+		}
+
 		/* Insert any necessary delays */
 		if (delay >= 1000) {
 			mdelay(delay / 1000);
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 499e437..cd204b1 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -87,6 +87,13 @@ static void of_get_regulation_constraints(struct device_node *np,
 			constraints->ramp_disable = true;
 	}
 
+	if (constraints->ramp_delay) {
+		ret = of_property_read_u32(np, "regulator-ramp-delay-scale",
+					   &pval);
+		if (!ret)
+			constraints->ramp_delay_scale = pval;
+	}
+
 	ret = of_property_read_u32(np, "regulator-enable-ramp-delay", &pval);
 	if (!ret)
 		constraints->enable_time = pval;
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index a1067d0..5153e5a 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -100,6 +100,12 @@ 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)
+ * @ramp_delay_scale: Scaling in ramp delay to provide extra guard time
+ *		      to settle down after voltage change. This is
+ *		      provided in terms of percentage like 200% means
+ *		      2x of calculated ramp delay for given voltage change.
+ *		      The final delay is calucated as:
+			DIV_ROUND_UP(ramp_delay * ramp_delay_scale, 100);
  * @enable_time: Turn-on time of the rails (unit: microseconds)
  */
 struct regulation_constraints {
@@ -139,6 +145,7 @@ struct regulation_constraints {
 
 	unsigned int ramp_delay;
 	unsigned int enable_time;
+	unsigned int ramp_delay_scale;
 
 	/* constraint flags */
 	unsigned always_on:1;	/* regulator never off when system is on */
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1346033 — Re: [PATCH 1/2] regulator: DT: Add support to scale ramp delay based on platform behavior

On Mon 29 Feb 06:40 PST 2016, Laxman Dewangan wrote:

> It is observed that voltage change in given rail affected by the load
> and the capacitor in the rail. This may cause the slow ramp in voltage
> against what PMIC has programmed.
> 
> The property regulator-ramp-delay provides the ramp delay configuration
> for PMIC but actual voltage settling time may change based on platform.
> 
> Add new property "regulator-ramp-delay-scale" for platform specific scaling
> in final ramp delay calculation. On this case, final wait delay time
> for voltage change is calculated as the
>    DIV_ROUND_UP(ramp_delay * ramp_delay_scale, 100)
> 
> The value is provided in the term of percentage i.e. 300% means it will
> do the 3x delay of calculated value.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---

The regulator-ramp-delay is a variable you can tweak on a board basis,
so I'm not sure what benefit it gives to be able to add a scaling
factor to this.

In my experience your HW engineer will say "you have to wait X ms", not
"you have to wait 125% of X ms".


Can you please elaborate on why the original knob isn't sufficient?

Regards,
Bjorn

>  Documentation/devicetree/bindings/regulator/regulator.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
> index 1d112fc..f2e8360 100644
> --- a/Documentation/devicetree/bindings/regulator/regulator.txt
> +++ b/Documentation/devicetree/bindings/regulator/regulator.txt
> @@ -16,6 +16,13 @@ Optional properties:
>  - regulator-ramp-delay: ramp delay for regulator(in uV/uS)
>    For hardware which supports disabling ramp rate, it should be explicitly
>    initialised to zero (regulator-ramp-delay = <0>) for disabling ramp delay.
> +- regulator-ramp-delay-scale: Platform specific scaling in ramp delay to
> +  provide extra guard time to settle down voltage after its change. This will
> +  help to wait for extra time for voltage settling based on platform behavior.
> +  This is provided in terms of percentage scaling like 300% means final delay
> +  will become 3x of calculated ramp delay for voltage change.
> +  The final delay is calculated as:
> +	delay = DIV_ROUND_UP(ramp-delay * ramp-delay-scale, 100)
>  - regulator-enable-ramp-delay: The time taken, in microseconds, for the supply
>    rail to reach the target voltage, plus/minus whatever tolerance the board
>    design requires. This property describes the total system ramp time
> -- 
> 2.1.4
> 

[toc] | [prev] | [next] | [standalone]

#1346331 — Re: [PATCH 1/2] regulator: DT: Add support to scale ramp delay based on platform behavior

[Multipart message — attachments visible in raw view] — view raw

On Mon, Feb 29, 2016 at 09:47:51AM -0800, Bjorn Andersson wrote:
> On Mon 29 Feb 06:40 PST 2016, Laxman Dewangan wrote:

> > It is observed that voltage change in given rail affected by the load
> > and the capacitor in the rail. This may cause the slow ramp in voltage
> > against what PMIC has programmed.

> The regulator-ramp-delay is a variable you can tweak on a board basis,
> so I'm not sure what benefit it gives to be able to add a scaling
> factor to this.

> In my experience your HW engineer will say "you have to wait X ms", not
> "you have to wait 125% of X ms".

> Can you please elaborate on why the original knob isn't sufficient?

Right, this definitely feels like the wrong thing is being specified
here (and also like the PMIC might be going out of spec, possibly as a
result of being overloaded) and that the existing board specific
controls should be used.  It just doesn't correspond to the way people
usually talk about specs for PMICs.

[toc] | [prev] | [next] | [standalone]

#1346352 — Re: [PATCH 1/2] regulator: DT: Add support to scale ramp delay based on platform behavior

On Tuesday 01 March 2016 07:53 AM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Mon, Feb 29, 2016 at 09:47:51AM -0800, Bjorn Andersson wrote:
>> On Mon 29 Feb 06:40 PST 2016, Laxman Dewangan wrote:
>>> It is observed that voltage change in given rail affected by the load
>>> and the capacitor in the rail. This may cause the slow ramp in voltage
>>> against what PMIC has programmed.
>> The regulator-ramp-delay is a variable you can tweak on a board basis,
>> so I'm not sure what benefit it gives to be able to add a scaling
>> factor to this.
>> In my experience your HW engineer will say "you have to wait X ms", not
>> "you have to wait 125% of X ms".
>> Can you please elaborate on why the original knob isn't sufficient?
> Right, this definitely feels like the wrong thing is being specified
> here (and also like the PMIC might be going out of spec, possibly as a
> result of being overloaded) and that the existing board specific
> controls should be used.  It just doesn't correspond to the way people
> usually talk about specs for PMICs.
>

Most of PMICs offer to configure the slew rate (ramp time). For 
discussion, I am considering MAX77620 LDOs provides option to 27mV/us 
and 100mV/us.

HW team characterize the board and its rail and come up with the 
following data:
- Configure PMIC to 27mV/us for ramp time.
- With this measured value of ramp on board is 10mV/us and it is safe 
to  assume 5mv/us to consider the board variations.

So we have now two input from HW team:
1. What should be configure in PMIC.
2. And for calculation, how much ramp need to be consider.

For (1), it is 25mV/us and for (2) which 540% (27 *100/5).

Currently, we can provide the 27mv/us as ramp-delay but do not have 
option for scaling it.

My patch add for knob for (2).

[toc] | [prev] | [next] | [standalone]

#1347761 — Re: [PATCH 1/2] regulator: DT: Add support to scale ramp delay based on platform behavior

[Multipart message — attachments visible in raw view] — view raw

On Tue, Mar 01, 2016 at 09:18:46AM +0530, Laxman Dewangan wrote:

> HW team characterize the board and its rail and come up with the following
> data:
> - Configure PMIC to 27mV/us for ramp time.
> - With this measured value of ramp on board is 10mV/us and it is safe to
> assume 5mv/us to consider the board variations.

> So we have now two input from HW team:
> 1. What should be configure in PMIC.
> 2. And for calculation, how much ramp need to be consider.

> For (1), it is 25mV/us and for (2) which 540% (27 *100/5).

> Currently, we can provide the 27mv/us as ramp-delay but do not have option
> for scaling it.

You're not trying to scale the value here, you're trying to replace the
value because the PMIC is incapable of delivering the advertised ramp
rate.  Trying to express this as a multiple of the advertised ramp rate
is just adding complexity.

[toc] | [prev] | [next] | [standalone]

#1347766 — Re: [PATCH 1/2] regulator: DT: Add support to scale ramp delay based on platform behavior

On Wednesday 02 March 2016 09:08 AM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Tue, Mar 01, 2016 at 09:18:46AM +0530, Laxman Dewangan wrote:
>
>> HW team characterize the board and its rail and come up with the following
>> data:
>> - Configure PMIC to 27mV/us for ramp time.
>> - With this measured value of ramp on board is 10mV/us and it is safe to
>> assume 5mv/us to consider the board variations.
>> So we have now two input from HW team:
>> 1. What should be configure in PMIC.
>> 2. And for calculation, how much ramp need to be consider.
>> For (1), it is 25mV/us and for (2) which 540% (27 *100/5).
>> Currently, we can provide the 27mv/us as ramp-delay but do not have option
>> for scaling it.
> You're not trying to scale the value here, you're trying to replace the
> value because the PMIC is incapable of delivering the advertised ramp
> rate.  Trying to express this as a multiple of the advertised ramp rate
> is just adding complexity.
>
So should we provide absolute ramp value here for platform specific?
Or any other suggestion to handle this situation as this is very common 
and almost all our boards have this slowness on ramp.

[toc] | [prev] | [next] | [standalone]

#1347779 — Re: [PATCH 1/2] regulator: DT: Add support to scale ramp delay based on platform behavior

[Multipart message — attachments visible in raw view] — view raw

On Wed, Mar 02, 2016 at 09:05:26AM +0530, Laxman Dewangan wrote:
> On Wednesday 02 March 2016 09:08 AM, Mark Brown wrote:

> >You're not trying to scale the value here, you're trying to replace the
> >value because the PMIC is incapable of delivering the advertised ramp
> >rate.  Trying to express this as a multiple of the advertised ramp rate
> >is just adding complexity.

> So should we provide absolute ramp value here for platform specific?

Yes, otherwise if the PMIC vendor respecifies their ramp rates to
reflect reality and the driver is updated then your DT will be broken.

> Or any other suggestion to handle this situation as this is very common and
> almost all our boards have this slowness on ramp.

Perhaps time to have a chat with your PMIC vendors...

[toc] | [prev] | [next] | [standalone]

#1347803 — Re: [PATCH 1/2] regulator: DT: Add support to scale ramp delay based on platform behavior

On Wednesday 02 March 2016 10:05 AM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Wed, Mar 02, 2016 at 09:05:26AM +0530, Laxman Dewangan wrote:
>> On Wednesday 02 March 2016 09:08 AM, Mark Brown wrote:
>>> You're not trying to scale the value here, you're trying to replace the
>>> value because the PMIC is incapable of delivering the advertised ramp
>>> rate.  Trying to express this as a multiple of the advertised ramp rate
>>> is just adding complexity.
>> So should we provide absolute ramp value here for platform specific?
> Yes, otherwise if the PMIC vendor respecifies their ramp rates to
> reflect reality and the driver is updated then your DT will be broken.

Then can we have other member for pmic ramp time register configuration 
via DT.
So measured value on platform will be existing ramp-delay and the value 
which will be configured in PMIC will be via pmic-ramp-delay if provided 
otherwise use ramp-delay.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web