On 16/07/2017 at 12:22:08 +0200, Rafael J. Wysocki wrote:

On Saturday, July 15, 2017 07:36:21 PM Florian Fainelli wrote:

quoted

On 07/15/2017 04:38 PM, Rafael J. Wysocki wrote:

quoted

On Sunday, July 16, 2017 01:34:53 AM Mason wrote:

quoted

On 16/07/2017 01:24, Rafael J. Wysocki wrote:

quoted

On Saturday, July 15, 2017 10:20:27 AM Florian Fainelli wrote:

quoted

The enum offers the advantage of centralizing how many different states
exist for all the platforms we know about in the kernel, it's easy to
define common values for platforms that have the same semantics, just
like it's simple to add new values for platform specific details.

Well, you seem to be liking this, so why don't you just implement it?

At the end of his message, Florian wrote:

quoted

In any case, just agree and I will be happy to follow-up with patches.

But it may be hard to convince everybody without posting code changes
and often enough showing a patch makes a good argument.

I had the patches ready last night, saw the emails this morning and
decided to go mountain bike for a bit to think about it some more. You
will find my follow-up patches that hopefully implement your recommendation.

OK, thanks!

There is one problem with this I missed before, though, sorry about that.

Drivers need to be able to distinguish between suspend-to-idle and the platform
states too, so we need to store the argument passed to suspend_devices_and_enter()
somewhere too, either in the core or in the platform code.

And if we need to store it anyway, let's just store it in the core in a global var
(say pm_suspend_target_state), export that and be done.

There still will be a concern regarding drivers that care about differences between
PM_SUSPEND_MEM and PM_SUSPEND_STANDBY, because those differences are
platform-dependent, but let's defer addressing this until we have a driver
that needs to run on different platforms with different definitions for those
things.

We already have the case for drivers/net/ethernet/cadence/ and
drivers/net/can/m_can/ and many of the at91 drivers. Depending on the
specific SoC they run on, PM_SUSPEND_MEM may or may not cut VDDcore or
may or may not change the peripheral clock.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com