>> The below patch makes sure that dev->parent supports runtime PM before accessing its PM fields. Though the code does this check at a later point in time, doing it earlier > can optimize the code flow.
> This is not a good idea. Those fields should be set correctly even if
> the parent's disable_depth is positive. What would happen if the
> fields were wrong and then somebody called pm_runtime_enable(parent)?
Yes Alan, those fields do get set but if you look at __pm_runtime_set_status function -
if (status == RPM_SUSPENDED) {
/* It always is possible to set the status to 'suspended'. */
if (parent) {
atomic_add_unless(&parent->power.child_count, -1, 0);
notify_parent = !parent->power.ignore_children;
}
goto out_set;
}
This code gets executed without doing any check to see if the parent supports runtime PM. Doing explicit check before can avoid executing these code at runtime.
> Whether this change optimizes the code flow is not entirely clear.
> Sure, it speeds things up a little in the case where the parent is
> disabled. But it also slows things down in the case where the parent
> is enabled.
As I mentioned the checks do happen in the code at a later point in time so these are not extra checks which will slow things when parent is enabled. But pulling this check up will avoid lot many other PM fields (like lock etc) access when runtime is not enabled, which I think is optimization.
Regards,
Nithish Mahalingam
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
