On Sat 2017-07-15 20:33:58, Alexandre Belloni wrote:

On 15/07/2017 at 10:20:27 -0700, Florian Fainelli wrote:

quoted

quoted

We already have

struct regulator_state {
       int uV; /* suspend voltage */
       unsigned int mode; /* suspend regulator operating mode */
       int enabled; /* is regulator enabled in this suspend state */
       int disabled; /* is the regulator disabled in this suspend state */
};

 * struct regulation_constraints - regulator operating constraints.
  * @state_disk: State for regulator when system is suspended in disk
  * mode.
  * @state_mem: State for regulator when system is suspended in mem
  * mode.
  * @state_standby: State for regulator when system is suspended in
  * standby
  *                 mode.
   
. So it seems that maybe we should tell the drivers if we are entering
"state_mem" or "state_standby" (something I may have opposed, sorry),
then the driver can get neccessary information from regulator
framework.

OK, so what would be the mechanism to tell these drivers about the
system wide suspend state they are entering if it is not via
platform_suspend_target_state()?

Keep in mind that regulators might be one aspect of what could be
causing the platform to behave specifically in one suspend state vs.
another, but there could be pieces of HW within the SoC that can't be
described with power domains, voltage islands etc. that would still have
inherent suspend states properties (like memory retention, pin/pad
controls etc. etc). We still need some mechanism, possibly centralized

I concur, the regulator stuff is one aspect of one of our suspend state
(cutting VDDcore). But we have another state where the main clock (going
to the IPs) is going from a few hundred MHz to 32kHz. This is currently
handled by calling at91_suspend_entering_slow_clock(). I think it is
important to take that into account so we can remove this hack from the
kernel.

Cure should not be worse then the disease... and it is in this case.

For clocks, take a look at clock framework, perhaps it already has "clock_will_be_suspended"
as regulator framework had. If not, implement it.

Same with memory retention, pin/pad controls.

									Pavel