On 09/26/2017 07:16 AM, Charles Keepax wrote:

On Fri, Sep 22, 2017 at 09:57:22AM -0700, Florian Fainelli wrote:

quoted

On 09/22/2017 06:20 AM, Charles Keepax wrote:

quoted

On Fri, Sep 22, 2017 at 01:55:22PM +0200, Linus Walleij wrote:

quoted

On Thu, Sep 21, 2017 at 3:04 AM, Florian Fainelli [off-list ref] wrote:

I guess in our case we didn't really consider the restore aspect
because we essentially get that for free from regmap. Regmap
cache's all our register state and provides a mechanism to sync
that back to the hardware, so we simply invoke that on resume and
all our GPIO/pinctrl state is restored.

As you may see, the problem in my case is that the hardware has only onw
pinctrl state: "default", and it loses its hardware register contents,
and because of early check in pinctrl_select_state(), we do nothing (the
state has not changed per-se), so we are left with SW thinking we
applied the "default" state again, while in fact we did not.

That is exactly the situation we have on the CODECs when they go
into runtime suspend, power is removed, and everything is back at
defaults when we resume. Just in our case we re-apply the state
as part of the CODEC resume using a regmap sync.

Do you just re-apply the previous state, or do you force a "fake" state
by moving to a state different than the current during suspend, just to
force a transition during resume?

quoted

The approach taken here was to move this to the core pinctrl code
because this is not something a pinctrl consumer should be aware of,
when it calls pinctrl_select_state(), it should do what it asked for.

Apologies if I have missed something here, but does the consumer
not still to some extent need to be aware with this solution
since it needs to re-request the pin state in resume?

Consumers may indeed have to call pinctrl_select_state() but because of
the current check that does:

if (p->state == state)

this is not happening, but you are absolutely right, consumers that wish
to see their pin state be (re)configured during driver resume absolutely
need to tell the core about it, I am not thinking about any of this
happening "under the hood", this absolutely would not be right.

I think that is really my only reservation here, is it feels
like this should be something that is purely implemented on the
provider, and be invisible to the consumer, and I am not clear
this is.

quoted

I also decided to make this a per-provider property as opposed to a
per-group property because chances are that the state retention is on a
per-controller basis, and not per-bank/group, although I may be wrong.

It seems quite likely that this property would mostly be
per-provider to me as well.

Thanks,
Charles

-- 
Florian