On 2017-10-25 at 00:16:25 -0700, Ramesh Thomas wrote:
> On 2017-10-24 at 13:35:05 +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wyso...@intel.com>
> >
>
> [cut]
>
> > @@ -63,10 +60,14 @@ static bool default_suspend_ok(struct de
> >
> > spin_unlock_irqrestore(&dev->power.lock, flags);
> >
> > - if (constraint_ns < 0)
> > + if (constraint_ns == 0)
> > return false;
> >
> > - constraint_ns *= NSEC_PER_USEC;
> > + if (constraint_ns == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT)
> > + constraint_ns = -1;
> > + else
> > + constraint_ns *= NSEC_PER_USEC;
> > +
> > /*
> > * We can walk the children without any additional locking, because
> > * they all have been suspended at this point and their
> > @@ -76,14 +77,19 @@ static bool default_suspend_ok(struct de
> > device_for_each_child(dev, &constraint_ns,
> > dev_update_qos_constraint);
> >
> > - if (constraint_ns > 0) {
> > - constraint_ns -= td->suspend_latency_ns +
> > - td->resume_latency_ns;
> > - if (constraint_ns == 0)
> > - return false;
> > + if (constraint_ns < 0) {
> > + /* The children have no constraints. */
> > + td->effective_constraint_ns =
> > PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> > + td->cached_suspend_ok = true;
> > + } else {
> > + constraint_ns -= td->suspend_latency_ns + td->resume_latency_ns;
> > + if (constraint_ns > 0) {
> > + td->effective_constraint_ns = constraint_ns;
> > + td->cached_suspend_ok = true;
> > + } else {
> > + td->effective_constraint_ns = 0;
>
> If the resume latency constraint was increased after this,
> default_power_down_ok may not consider the new value. default_suspend_ok needs
> to get called first if the new value is to be read.
>
> This is because dev_pm_qos_read_value will get called only if
> effective_constraint_ns has a negative value. default_suspend_ok initializes
> effective_constraint_ns with -1 before doing the calculations.
> default_power_down_ok does not initialize it to -1 and uses
> the existing value.
>
> A comment in default_power_down_ok implies it is not necessary to call
> default_suspend_ok before calling default_power_down_ok. In that case,
> default_power_down_ok should be able to get the new latency constraint value.
>
The design expects default_suspend_ok would always be called before
default_power_down_ok if the device was made "active" after start. Changes
to resume latency constraint will not be considered if it happened between
suspend and power down of a device. However, that is the design and not a
behavior introduced by this patch.
Acked-by: Ramesh Thomas <ramesh.tho...@intel.com>
