Re: [PATCH v4 3/4] regulator: Add support for a fixed delay after voltage increases

[Rob Herring]

On Tue, Sep 06, 2016 at 12:05:04PM -0700, Matthias Kaehlcke wrote:
> The target voltage isn't necessarily reached inmediately after
> requesting a regulator to change the voltage. In some cases the
> ramp_delay can be used to calculate the stabilisation time, in others
> there is no direct relationship between the delta in the voltage and
> the stabilisation time. This change introduces the device tree property
> "regulator-settle-time-up-us" which allows to specify a fixed delay
> after a voltage increase.
> 
> We don't add an option of a fixed delay on the way down for now because
> the way down is probably modelled best with a ramp rate, not a fixed
> delay.
> 
> Signed-off-by: Matthias Kaehlcke 
> Signed-off-by: Douglas Anderson 
> 
> ---
> Changes in v4:
> - Moved from PWM regulator to regulator core
> - Added 'regulator' prefix to device tree property
> 
>  .../devicetree/bindings/regulator/regulator.txt|  2 ++
>  drivers/regulator/core.c   | 24 
> +-
>  drivers/regulator/of_regulator.c   |  4 
>  include/linux/regulator/machine.h  |  2 ++
>  4 files changed, 22 insertions(+), 10 deletions(-)

Acked-by: Rob Herring 

Re: [PATCH v4 3/4] regulator: Add support for a fixed delay after voltage increases

[Rob Herring]

On Tue, Sep 06, 2016 at 12:05:04PM -0700, Matthias Kaehlcke wrote:
> The target voltage isn't necessarily reached inmediately after
> requesting a regulator to change the voltage. In some cases the
> ramp_delay can be used to calculate the stabilisation time, in others
> there is no direct relationship between the delta in the voltage and
> the stabilisation time. This change introduces the device tree property
> "regulator-settle-time-up-us" which allows to specify a fixed delay
> after a voltage increase.
> 
> We don't add an option of a fixed delay on the way down for now because
> the way down is probably modelled best with a ramp rate, not a fixed
> delay.
> 
> Signed-off-by: Matthias Kaehlcke 
> Signed-off-by: Douglas Anderson 
> 
> ---
> Changes in v4:
> - Moved from PWM regulator to regulator core
> - Added 'regulator' prefix to device tree property
> 
>  .../devicetree/bindings/regulator/regulator.txt|  2 ++
>  drivers/regulator/core.c   | 24 
> +-
>  drivers/regulator/of_regulator.c   |  4 
>  include/linux/regulator/machine.h  |  2 ++
>  4 files changed, 22 insertions(+), 10 deletions(-)

Acked-by: Rob Herring 

Re: [PATCH v4 3/4] regulator: Add support for a fixed delay after voltage increases

[Doug Anderson]

Hi

On Tue, Sep 13, 2016 at 2:36 AM, Mark Brown  wrote:
> On Tue, Sep 06, 2016 at 12:05:04PM -0700, Matthias Kaehlcke wrote:
>
>> the stabilisation time. This change introduces the device tree property
>> "regulator-settle-time-up-us" which allows to specify a fixed delay
>
>> We don't add an option of a fixed delay on the way down for now because
>> the way down is probably modelled best with a ramp rate, not a fixed
>> delay.
>
> Are you sure?  It seems more likely to be highly load dependent if
> there's an appreciable variation.  That could be a ramp rate if you've
> got a very predictable load and a simple regulator but it seems like
> the system is less likely to care if we collapsed down in that case.

In the particular case we cared about we had to make some assumptions
about the slowest possible ramp rate downward because we were trying
to avoid triggering an over voltage protection.  In that particular
case the best way to model it was with a ramp rate because longer
voltage changes caused longer worst case transitions.  Just like you
said, the actual ramp time is highly variable but the "worst case
slowest" ramp time is best modeled with a ramp rate.

-Doug

Re: [PATCH v4 3/4] regulator: Add support for a fixed delay after voltage increases

[Doug Anderson]

Hi

On Tue, Sep 13, 2016 at 2:36 AM, Mark Brown  wrote:
> On Tue, Sep 06, 2016 at 12:05:04PM -0700, Matthias Kaehlcke wrote:
>
>> the stabilisation time. This change introduces the device tree property
>> "regulator-settle-time-up-us" which allows to specify a fixed delay
>
>> We don't add an option of a fixed delay on the way down for now because
>> the way down is probably modelled best with a ramp rate, not a fixed
>> delay.
>
> Are you sure?  It seems more likely to be highly load dependent if
> there's an appreciable variation.  That could be a ramp rate if you've
> got a very predictable load and a simple regulator but it seems like
> the system is less likely to care if we collapsed down in that case.

In the particular case we cared about we had to make some assumptions
about the slowest possible ramp rate downward because we were trying
to avoid triggering an over voltage protection.  In that particular
case the best way to model it was with a ramp rate because longer
voltage changes caused longer worst case transitions.  Just like you
said, the actual ramp time is highly variable but the "worst case
slowest" ramp time is best modeled with a ramp rate.

-Doug

Re: [PATCH v4 3/4] regulator: Add support for a fixed delay after voltage increases

[Mark Brown]

On Tue, Sep 06, 2016 at 12:05:04PM -0700, Matthias Kaehlcke wrote:

> the stabilisation time. This change introduces the device tree property
> "regulator-settle-time-up-us" which allows to specify a fixed delay

> We don't add an option of a fixed delay on the way down for now because
> the way down is probably modelled best with a ramp rate, not a fixed
> delay.

Are you sure?  It seems more likely to be highly load dependent if
there's an appreciable variation.  That could be a ramp rate if you've
got a very predictable load and a simple regulator but it seems like
the system is less likely to care if we collapsed down in that case.


signature.asc
Description: PGP signature

Re: [PATCH v4 3/4] regulator: Add support for a fixed delay after voltage increases

[Mark Brown]

On Tue, Sep 06, 2016 at 12:05:04PM -0700, Matthias Kaehlcke wrote:

> the stabilisation time. This change introduces the device tree property
> "regulator-settle-time-up-us" which allows to specify a fixed delay

> We don't add an option of a fixed delay on the way down for now because
> the way down is probably modelled best with a ramp rate, not a fixed
> delay.

Are you sure?  It seems more likely to be highly load dependent if
there's an appreciable variation.  That could be a ramp rate if you've
got a very predictable load and a simple regulator but it seems like
the system is less likely to care if we collapsed down in that case.


signature.asc
Description: PGP signature

[PATCH v4 3/4] regulator: Add support for a fixed delay after voltage increases

[Matthias Kaehlcke]

The target voltage isn't necessarily reached inmediately after
requesting a regulator to change the voltage. In some cases the
ramp_delay can be used to calculate the stabilisation time, in others
there is no direct relationship between the delta in the voltage and
the stabilisation time. This change introduces the device tree property
"regulator-settle-time-up-us" which allows to specify a fixed delay
after a voltage increase.

We don't add an option of a fixed delay on the way down for now because
the way down is probably modelled best with a ramp rate, not a fixed
delay.

Signed-off-by: Matthias Kaehlcke 
Signed-off-by: Douglas Anderson 

---
Changes in v4:
- Moved from PWM regulator to regulator core
- Added 'regulator' prefix to device tree property

 .../devicetree/bindings/regulator/regulator.txt|  2 ++
 drivers/regulator/core.c   | 24 +-
 drivers/regulator/of_regulator.c   |  4 
 include/linux/regulator/machine.h  |  2 ++
 4 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt 
b/Documentation/devicetree/bindings/regulator/regulator.txt
index ecfc593..d2d1c4d 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -21,6 +21,8 @@ Optional properties:
   design requires. This property describes the total system ramp time
   required due to the combination of internal ramping of the regulator itself,
   and board design issues such as trace capacitance and load on the supply.
+- regulator-settle-time-up-us: Time to settle down after a voltage increase
+  (unit: us). For regulators with a ramp delay the two values are added.
 - regulator-soft-start: Enable soft start so that voltage ramps slowly
 - regulator-state-mem sub-root node for Suspend-to-RAM mode
   : suspend to memory, the device goes to sleep, but all data stored in memory,
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b1cef47..ed69fdc 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2801,7 +2801,9 @@ static int _regulator_do_set_voltage(struct regulator_dev 
*rdev,
ret = -EINVAL;
}
 
-   if (ret != 0 || rdev->constraints->ramp_disable)
+   if (ret != 0 ||
+   (rdev->constraints->ramp_disable &&
+   !rdev->constraints->settle_time_up))
goto no_delay;
 
if (rdev->desc->ops->set_voltage_time) {
@@ -3050,25 +3052,27 @@ EXPORT_SYMBOL_GPL(regulator_set_voltage_time);
  * Provided with the starting and ending voltage, this function calculates
  * the time in microseconds required to rise or fall to this new voltage.
  *
- * Drivers providing ramp_delay in regulation_constraints can use this as their
- * set_voltage_time() operation.
+ * Drivers providing ramp_delay or settle_time_up in regulation_constraints
+ * can use this as their set_voltage_time() operation.
  */
 int regulator_set_voltage_time_op(struct regulator_dev *rdev,
int old_uV, int new_uV)
 {
+   unsigned int settle_time = 0;
unsigned int ramp_delay = 0;
 
+   if (new_uV > old_uV)
+   settle_time = rdev->constraints->settle_time_up;
+
if (rdev->constraints->ramp_delay)
ramp_delay = rdev->constraints->ramp_delay;
else if (rdev->desc->ramp_delay)
ramp_delay = rdev->desc->ramp_delay;
 
-   if (ramp_delay == 0) {
-   rdev_warn(rdev, "ramp_delay not set\n");
-   return 0;
-   }
+   if (ramp_delay == 0)
+   return settle_time;
 
-   return DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);
+   return settle_time + DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);
 }
 EXPORT_SYMBOL_GPL(regulator_set_voltage_time_op);
 
@@ -3081,8 +3085,8 @@ EXPORT_SYMBOL_GPL(regulator_set_voltage_time_op);
  * Provided with the starting and target voltage selectors, this function
  * returns time in microseconds required to rise or fall to this new voltage
  *
- * Drivers providing ramp_delay in regulation_constraints can use this as their
- * set_voltage_time_sel() operation.
+ * Drivers providing ramp_delay or settle_time_up in regulation_constraints
+ * can use this as their set_voltage_time_sel() operation.
  */
 int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
   unsigned int old_selector,
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 4f613ec..056f242 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -90,6 +90,10 @@ static void of_get_regulation_constraints(struct device_node 
*np,
if (!ret)
constraints->enable_time = pval;
 
+   ret = of_property_read_u32(np, "regulator-settle-time-up-us", 

[PATCH v4 3/4] regulator: Add support for a fixed delay after voltage increases

[Matthias Kaehlcke]

The target voltage isn't necessarily reached inmediately after
requesting a regulator to change the voltage. In some cases the
ramp_delay can be used to calculate the stabilisation time, in others
there is no direct relationship between the delta in the voltage and
the stabilisation time. This change introduces the device tree property
"regulator-settle-time-up-us" which allows to specify a fixed delay
after a voltage increase.

We don't add an option of a fixed delay on the way down for now because
the way down is probably modelled best with a ramp rate, not a fixed
delay.

Signed-off-by: Matthias Kaehlcke 
Signed-off-by: Douglas Anderson 

---
Changes in v4:
- Moved from PWM regulator to regulator core
- Added 'regulator' prefix to device tree property

 .../devicetree/bindings/regulator/regulator.txt|  2 ++
 drivers/regulator/core.c   | 24 +-
 drivers/regulator/of_regulator.c   |  4 
 include/linux/regulator/machine.h  |  2 ++
 4 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt 
b/Documentation/devicetree/bindings/regulator/regulator.txt
index ecfc593..d2d1c4d 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -21,6 +21,8 @@ Optional properties:
   design requires. This property describes the total system ramp time
   required due to the combination of internal ramping of the regulator itself,
   and board design issues such as trace capacitance and load on the supply.
+- regulator-settle-time-up-us: Time to settle down after a voltage increase
+  (unit: us). For regulators with a ramp delay the two values are added.
 - regulator-soft-start: Enable soft start so that voltage ramps slowly
 - regulator-state-mem sub-root node for Suspend-to-RAM mode
   : suspend to memory, the device goes to sleep, but all data stored in memory,
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b1cef47..ed69fdc 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2801,7 +2801,9 @@ static int _regulator_do_set_voltage(struct regulator_dev 
*rdev,
ret = -EINVAL;
}
 
-   if (ret != 0 || rdev->constraints->ramp_disable)
+   if (ret != 0 ||
+   (rdev->constraints->ramp_disable &&
+   !rdev->constraints->settle_time_up))
goto no_delay;
 
if (rdev->desc->ops->set_voltage_time) {
@@ -3050,25 +3052,27 @@ EXPORT_SYMBOL_GPL(regulator_set_voltage_time);
  * Provided with the starting and ending voltage, this function calculates
  * the time in microseconds required to rise or fall to this new voltage.
  *
- * Drivers providing ramp_delay in regulation_constraints can use this as their
- * set_voltage_time() operation.
+ * Drivers providing ramp_delay or settle_time_up in regulation_constraints
+ * can use this as their set_voltage_time() operation.
  */
 int regulator_set_voltage_time_op(struct regulator_dev *rdev,
int old_uV, int new_uV)
 {
+   unsigned int settle_time = 0;
unsigned int ramp_delay = 0;
 
+   if (new_uV > old_uV)
+   settle_time = rdev->constraints->settle_time_up;
+
if (rdev->constraints->ramp_delay)
ramp_delay = rdev->constraints->ramp_delay;
else if (rdev->desc->ramp_delay)
ramp_delay = rdev->desc->ramp_delay;
 
-   if (ramp_delay == 0) {
-   rdev_warn(rdev, "ramp_delay not set\n");
-   return 0;
-   }
+   if (ramp_delay == 0)
+   return settle_time;
 
-   return DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);
+   return settle_time + DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);
 }
 EXPORT_SYMBOL_GPL(regulator_set_voltage_time_op);
 
@@ -3081,8 +3085,8 @@ EXPORT_SYMBOL_GPL(regulator_set_voltage_time_op);
  * Provided with the starting and target voltage selectors, this function
  * returns time in microseconds required to rise or fall to this new voltage
  *
- * Drivers providing ramp_delay in regulation_constraints can use this as their
- * set_voltage_time_sel() operation.
+ * Drivers providing ramp_delay or settle_time_up in regulation_constraints
+ * can use this as their set_voltage_time_sel() operation.
  */
 int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
   unsigned int old_selector,
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 4f613ec..056f242 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -90,6 +90,10 @@ static void of_get_regulation_constraints(struct device_node 
*np,
if (!ret)
constraints->enable_time = pval;
 
+   ret = of_property_read_u32(np, "regulator-settle-time-up-us", );
+   if (!ret)
+