Re: [PATCH fixes v3] pinctrl: Really force states during suspend/resume

From: Linus Walleij <linus.walleij@linaro.org>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Stephen Warren <swarren@nvidia.com>,
	Al Cooper <alcooperx@gmail.com>,
	"open list:PIN CONTROL SUBSYSTEM" <linux-gpio@vger.kernel.org>
Subject: Re: [PATCH fixes v3] pinctrl: Really force states during suspend/resume
Date: Thu, 16 Mar 2017 15:08:40 +0100	[thread overview]
Message-ID: <CACRpkdZBrYT=vXhR77W+v_hN=t6zYACVPm5HR==kC7nmDYJLQA@mail.gmail.com> (raw)
In-Reply-To: <18ca4418-8b62-a1c9-1282-0c468f2aefb2@gmail.com>

On Wed, Mar 15, 2017 at 3:18 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 03/14/2017 03:16 AM, Linus Walleij wrote:

>> The most obvious would be to use the API as many already do:
>> define "sleep" states in the core, and switch to these before
>> going to sleep. If CONFIG_PM is available simply by calling
>> pinctrl_pm_select_sleep_state() in the driver suspend() callback.
>
> Well, the difficulty for our platforms is that S2 does not make the HW
> lose pin states, only S3 does and drivers should be agnostic of S2 vs. S3.
>
> There is not really a "sleep" and "default" state defined for these
> platforms just the "default" state. I initially even considered adding a
> fake "sleep" state just to satisfy the state transition condition, but
> that does not accurately represent the HW.

Do you mean that on the way up, on the resume path, you know
whether the setting was lost or not?

Or you don't know it anywhere?

It is not less elegant to uncessesarily switch to a sleep state
than to unnecessarily program the default state when you only
went into S2 in that case.

I guess then it is better to assume we will loose the state, or
push for more granular handling of S2/3 etc states in the
PM core (I guess these states comes from ACPI or similar).

>> Alternatively we would add a function to set the pinctrl handle to
>> an "unknown" state, so that when we resume, the pinctrl core at
>> least knows that we are not in "default" state anymore, so that
>> "default" is applied.
>
> And such a function would be called during driver suspend? Would not we
> still end-up with the drivers having to know about the fact that there
> is a) only one pin state defined, and b) these pins potentially lose
> their states in some deep sleep mode?

Again, the proposal to switch to default state twice just because
we do not know how deep sleep we went into isn't any more
elegant. Then it is better to just assume we lost the state at
all times.

Alternatively develop the PM core. Is it really impossible for
PM hooks to know which state it went into/came from?

Yours,
Linus Walleij