On Monday, November 13, 2017 2:26:28 PM CET Ulf Hansson wrote:

On 12 November 2017 at 01:27, Rafael J. Wysocki [off-list ref] wrote:

quoted

From: Rafael J. Wysocki <redacted>

The check for "active" children in __pm_runtime_set_status(), when
trying to set the parent device status to "suspended", doesn't
really make sense, because in fact it is not invalid to set the
status of a device with runtime PM disabled to "suspended" in any
case.  It is invalid to enable runtime PM for a device with its
status set to "suspended" while its child_count reference counter
is nonzero, but the check in __pm_runtime_set_status() doesn't
really cover that situation.

The reason to why I changed this in commit a8636c89648a ("PM /
Runtime: Don't allow to suspend a device with an active child") was
because to get a consistent behavior.

Well, it causes the function to return an error in a non-error situation,
which IMnsHO is a bug.

Because, setting the device's status to active (RPM_ACTIVE) via
__pm_runtime_set_status(), requires its parent to also be active (in
case the parent has runtime PM enabled).

No!!!

The check is in there, because the parent's usage_count is affected by that
code and incrementing it in case the parent has runtime PM enabled and is
RPM_SUSPENDED leads to an inconsistent runtime PM state of the parent.  IOW,
it would confuse the framework.

There's no such issue if the runtime PM status of a child is set to RPM_SUSPENDED.

It is all consistent without the check I'm removing and is made inconsistent
by that very check.

I would prefer to try maintain this consistency, but I also I realize
that commit a8636c89648a, should also have been checking if the parent
has runtime PM enabled (again for consistency), which it doesn't.

What about fixing that instead?

Runtime PM is *disabled* for the parent at this point, guaranteed, so there's
nothing to check, I'm afraid ...

OTOH, the runtime PM status of the children doesn't matter here, because their
reference counters are not updated.

Thanks,
Rafael