Re: [PATCH] ARM: OMAP2+: omap_device: maintain sane runtime pmstatus around suspend/resume

[Nishanth Menon]

On 16:38-20131107, Kevin Hilman wrote:
[...]
> That's debatable I guess.  The ideal world is that runtime PM hides all
> of this, but I'm not sure it's achievable in all cases.
> 
Agreed. some drivers like edma need to save and restore context around
suspend.
[...]

> No, that sysfs knob is for disabling runtime PM.  We still want the
> device to hit low-power state in system suspend.  Solving that problem
> is half the reason we have this omap_device noirq mess in the first
> place.
> 
> You need to test this by disabling runtime PM from userspace and ensure
> that the low power state is still hit during suspend.
> 
Done and it still does work, makes sense since it just ensures that
runtime PM's dev->power.runtime_status is set to RPM_SUSPENDED instead
of RPM_ACTIVE for devices that depend on autosuspend.

Logs (based on vendor kernel which has relevant out of tree patches to
enable suspend resume - still in the works):
AM335x-BBB: http://pastie.org/8472182
OMAP5-uEVM: http://pastie.org/8472183

> >>
> >>> +				/* NOTE: *might* indicate driver race */
> >>
> >> Yes, a driver race which should then be fixed in the driver.
> >
> > true if this is a non-autosuspend device, in auto suspend devices,
> > this could be a regular phenomenon if timeout is pretty large.. but
> > atleast that should allow debug.
> 
> Agreed.  I wasn't thinking about the autosuspend case.  Thanks for
> clarifying.
> 
> >>
> >>> +				dev_dbg(dev, "%s: Force suspending\n",
> >>> +					__func__);
> >>> +				pm_runtime_set_suspended(dev);
> >>> +				od->flags |= OMAP_DEVICE_SUSPEND_FORCED;
> >>
> >> Not sure why you need an additonal flag.  Why not just always do this
> >> and use the existin OMAP_DEVICE_SUSPENDED flag.
> >
> > restore of runtime data structure state is only needed for specific
> > devices - not all..
> 
> The question is why do you a flag in addition to OMAP_DEVICE_SUSPEND.
> Whenever that flag is set, omap_device has gone behind your back, and
> the runtime PM status should be kept in sync.

Yes, you are right, originally, I had intended this to indicate devices
that needed to be runtime_status updated, but then, now I realize that
it is true for all devices that have OMAP_DEVICE_SUSPEND set. It can be
applied without an additional flag. Do see if the updated patch is more
sensible:
-- >8 --