From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Fainelli Subject: Re: [PATCH 1/2] pinctrl: Allow a device to indicate when to force a state Date: Tue, 26 Sep 2017 10:51:14 -0700 Message-ID: <61d19678-e90c-3b3a-099a-81200873edc8@gmail.com> References: <20170921010421.7467-1-f.fainelli@gmail.com> <20170921010421.7467-2-f.fainelli@gmail.com> <20170922132011.wssbhnsggteffgte@localhost.localdomain> <20170926141628.h27hcrujzhxu4sph@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from mail-qk0-f195.google.com ([209.85.220.195]:33465 "EHLO mail-qk0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965587AbdIZRvW (ORCPT ); Tue, 26 Sep 2017 13:51:22 -0400 In-Reply-To: <20170926141628.h27hcrujzhxu4sph@localhost.localdomain> Content-Language: en-US Sender: linux-gpio-owner@vger.kernel.org List-Id: linux-gpio@vger.kernel.org To: Charles Keepax Cc: Linus Walleij , ext Tony Lindgren , Charles Keepax , "linux-kernel@vger.kernel.org" , Stephen Warren , Andy Shevchenko , Al Cooper , "linux-gpio@vger.kernel.org" , "devicetree@vger.kernel.org" , Rob Herring , Mark Rutland , bcm-kernel-feedback-list 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 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