On Thursday, June 22, 2017 10:51:02 AM Alexandre Belloni wrote:

On 21/06/2017 at 16:55:04 -0700, Florian Fainelli wrote:

quoted

quoted

That is conceivable, but again, the meaning of STANDBY and MEM is
platform-specific.  Actions to be taken for, say, STANDBY, may differ
from one platform to another.

True, though I would only expect drivers for that particular platform to
care about that information and these drivers would only make sense on
that particular platform so the meaning of STANDBY and MEM would be
clear for those drivers. The intent is really to keep the "distributed"
model where individual drivers manage their particular piece of HW,
while utilizing a global piece of information that is platform specific.

If this seems acceptable to you along with proper documentation to
illustrate the platform specific meaning of these states, I will got
ahead and cook a patch.

Well, that won't work for us. We need need two kind of information:
 - whether main clock is switched from the master clock to the slow
   clock
 - whether VDDcore will be cut

for the first one, we already have an hackish callback:
at91_suspend_entering_slow_clock() that will call from the platform
specific drivers.

The main issue now is for the second one. We also need it for IPs that
are shared with other SoCs. For example, the macb ethernet controller
that is shared with the zynq and the m_can controller. In those cases,
I don't think it is a good idea to add platform specific code in the
drivers but at the same time, I don't think it is reasonable to
unconditionally reinit the IP on resume as there is a significant
latency hit. I don't think that the xilinx guys will like when suddenly
their platform takes 500ms to resume.

We can add a ->target_state callback in struct platform_suspend_ops that
will return a value representing the the particular target state the platform
has chosen to go to.  Along with it, there can be a list of values for all
of the low-level sleep states defined by all of the different platforms
(in which case I'd like to reserve values 0-5 for the ACPI system states).

There can be a wrapper, say platform_suspend_target_state() around that,
so the driver will call it, compare the return value with what is known to it
about the different platforms it can run on and act accordingly.

Thanks,
Rafael