On 09/26/2017 07:16 AM, Charles Keepax wrote: > On Fri, Sep 22, 2017 at 09:57:22AM -0700, Florian Fainelli wrote: >> On 09/22/2017 06:20 AM, Charles Keepax wrote: >>> On Fri, Sep 22, 2017 at 01:55:22PM +0200, Linus Walleij wrote: >>>> On Thu, Sep 21, 2017 at 3:04 AM, Florian Fainelli <f.fainelli@xxxxxxxxx> 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? > >> 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. > >> 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html