Re: [PATCH v1] PM: sleep: core: Synchronize runtime PM status of parents and children

[Ulf Hansson]

[...]

> >
> > The problem with $subject patch is that drivers/buses are required to
> > check the runtime PM status, with pm_runtime_suspended(),
> > pm_runtime_status_suspended() or pm_runtime_active() in one of its
> > system suspend/resume callbacks , to synchronize it with the HW state
> > for its device (turn on/off clocks for example).
>
> Well, I'm kind of unaware of this requirement.
>
> It clearly is not even followed by the code without the $subject patch.
>
> The real requirement is that the runtime PM status at the point when
> runtime PM is re-enabled, that is in device_resume_early(), must
> reflect the current actual HW state.

Right. Seems like we are in agreement, just that there seems to be
multiple ways to describe the similar problem.

>
> > Certainly, we can not rely on drivers to conform to this behaviour and
> > there are plenty of cases where they really don't. For example, we
> > have drivers that only implements runtime PM support or simply don't
> > care about the runtime PM status during system resume, but just leaves
> > the device in the state it is already in.
>
> Drivers that only support runtime PM are broken with respect to system
> sleep ATM.  They need to be made to support system sleep or they
> cannot be used on systems that use system sleep.  There may be a way
> around this for system suspend/resume (see below), but not for
> hibernation.

I think calling them broken may be to take this a step too far.

While I certainly agree that these drivers have room for some
improvements, it looks to me that these drivers work today, but may
not with $subject patch.

>
> > Moreover, we have the users of pm_runtime_force_suspend|resume(),
> > which we also know *not* to work with DPM_FLAG_SMART_SUSPEND and thus
> > $subject patch too. I am less worried about these cases though, as I
> > believe we should be able to fix them, by taking into account the
> > suggested "->power.set_active flag", or something along those lines.
>
> Yes, and that's what I'm going to do.
>
> > That said, it seems like we need another way forward.
>
> I still don't see why, sorry.
>
> I guess the concern is that if a device suddenly needs to be resumed
> during system resume even though it was runtime-suspended before the
> preceding system suspend, there is no way to tell its driver/bus
> type/etc that this is the case if they all decide to leave the device
> as is, but as I have said for multiple times in this thread, leaving a
> device as is during system resume may not be an option unless it is a
> leaf device without any subordinates.  This has always been the case.
>
> We'll see if there is any damage resulting from the $subject change
> and we'll fix it if so.
>
> In the future, though, I'd like to integrate system resume with
> runtime PM more than it is now.  Namely, it should be possible to move
> the re-enabling of runtime PM to the front of device_resume_early()
> and then use pm_runtime_resume() for resuming devices whose drivers
> support runtime PM (I don't see any fundamental obstacles to this).
> One benefit of doing this would be that pm_runtime_resume() would
> automatically trigger a runtime resume for all of the "superior"
> devices, so they could be left in whatever state they had been in
> before the preceding system suspend regardless of what happens to
> their "subordinates".  It is likely that some kind of driver opt-in
> will be needed for this or maybe the core can figure it out by itself.

Right, I am certainly interested to discuss this topic too. It's not
an easy thing and the biggest problem is that we can't really just
change the behaviour without a big risk of breaking things.

Some kind of opt-in behaviour is the only thing that can work, in my
opinion. Anyway, I am here to review and discuss. :-)

>
> It can look at what callbacks are implemented etc.  For example, if a
> driver only implements :runtime_suspend() and :runtime_resume() and no
> other PM callbacks, it is reasonable to assume that the devices
> handled by it should be suspended and resumed with the help of the
> runtime PM infrastructure even during system-wide suspend/resume (that
> doesn't apply to hibernation, though).

Maybe this can work in some opt-in/out way, but certainly not as a
solution for all.

For example, we have subsystems that deal with system suspend/resume
quite differently, where drivers instead implement some subsystem
specific callbacks, rather than the common system suspend/resume ops.

Kind regards
Uffe