Re: [PATCH 0/12] PM / sleep: Driver flags for system suspend/resume

[Rafael J. Wysocki]

On Tuesday, October 17, 2017 10:12:19 PM CEST Alan Stern wrote:
> On Tue, 17 Oct 2017, Ulf Hansson wrote:
> 
> > > These functions are wrong, however, because they attempt to reuse the
> > > whole callback *path* instead of just reusing driver callbacks.  The
> > > *only* reason why it all "works" is because there are no middle layer
> > > callbacks involved in that now.
> > >
> > > If you changed them to reuse driver callbacks only today, nothing would break
> > > AFAICS.
> > 
> > Yes, it would.
> > 
> > First, for example, the amba bus is responsible for the amba bus
> > clock, but relies on drivers to gate/ungate it during system sleep. In
> > case the amba drivers don't use the pm_runtime_force_suspend|resume(),
> > it will explicitly have to start manage the clock during system sleep
> > themselves. Leading to open coding.
> 
> I think what Rafael has in mind is that the PM core will call the amba
> bus's ->suspend callback, and that routine will then be able to call
> the amba driver's runtime_suspend routine directly, if it wants to --
> as opposed to going through pm_runtime_force_suspend.

Right in general.

> However, it's not clear whether this fully answers your concerns.

Well, in the particular AMBA case fixing this should be quite straightforward.

> > Second, it will introduce a regression in behavior for all users of
> > pm_runtime_force_suspend|resume(), especially during system resume as
> > the driver may then end up resuming the device even in case it isn't
> > needed. I believe I have explained why, also several times by now -
> > and that's also how far you could take the i2c designware driver at
> > this point.
> > 
> > That said, I assume the second part may be addressed in this series,
> > if these drivers convert to use the "driver PM flags", right?
> 
> Presumably.
> 
> The problem is how to handle things which need to be treated
> differently for runtime PM vs. system suspend vs. hibernation.  If
> everything filters through a runtime_suspend routine, that doesn't
> leave any scope for handling the different kinds of PM transitions
> differently.  Instead, we can make the middle layer (i.e., the bus-type
> callbacks) take care of the varying tasks, and they can directly invoke
> a driver's runtime-PM callbacks to handle all the common activities.  
> If that's how the middle layer wants to do it.

Well, that's what happens today, except that driver runtime PM callbacks
are not directly invoked.  Actually, I tried to implement that, but it was
so ugly and fragile that I gave up.

It really is better if drivers point the different callback pointers to the
same rountine if they want to reuse it.

> > However, what about the first case? Is some open coding needed or your
> > think the amba driver can instruct the amba bus via the "driver PM
> > flags"?
> 
> PM flags won't directly be able to cover things like disabling clocks.  
> But they could be useful for indicating explicitly whether the code to
> take care of those things needs to reside at the driver layer or at the
> bus layer.

Right.

Thanks,
Rafael