From mboxrd@z Thu Jan 1 00:00:00 1970 From: tony@atomide.com (Tony Lindgren) Date: Mon, 24 Jun 2013 03:10:55 -0700 Subject: [PATCH] pinctrl: document the pinctrl PM states In-Reply-To: <51C4A593.6000900@wwwdotorg.org> References: <1370980749-15383-1-git-send-email-linus.walleij@stericsson.com> <51BA1FE7.4000900@wwwdotorg.org> <51BB3A2C.608@wwwdotorg.org> <20130617072021.GB4465@atomide.com> <51C20E3A.8090304@wwwdotorg.org> <20130620063823.GD5523@atomide.com> <51C3577E.6040709@wwwdotorg.org> <20130621062552.GI5523@atomide.com> <51C4A593.6000900@wwwdotorg.org> Message-ID: <20130624101055.GT5523@atomide.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org * Stephen Warren [130621 12:18]: > On 06/21/2013 12:25 AM, Tony Lindgren wrote: > > * Stephen Warren [130620 12:32]: > >> > >> I assume you mean there shouldn't be any issue *modifying* the pinctrl > >> API to allow multiple states to be active at once? And where you're > >> talking about having multiple sets active at once already, you're > >> talking about some other API? > > > > Nope, the standard pinctrl API. At least I have not seen issues with > > having multiple states active the same time in a single driver. > > Please take a look at the implementation of pinctrl_select_state(). It > very explicitly performs the following steps: > > 1) Find all pins(groups) that are used in the current state but not the > new state, and execute pinctrl_disable_setting() on them. (For mux > settings only, not pin config, since the core doesn't have any idea how > to reverse config settings). > > 2) For all settings in the new state, apply those settings. > > So, it very explicitly only allows a single state to be set at a time. > Equally, p->state (the field which stores the currently selected state) > is a single item, not a set/list/array. OK thanks I get now what you're saying. I did not see the p->state issue as the disable function won't do anything for the SoCs that I mostly deal with. > So, this code needs rework if you want the core to support the concept > of having multiple states active at once, since it needs separate > pinctrl_activate_state() and pinctrl_deactivate_state() APIs, in order > to avoid step (1) above. And of course, p->state would need to be a > set/list/array. I'll think about it a bit and do a patch to fix this. It seems that that we need just two entries in the p->state array: static (default), and dynamic. Then the dynamic would be typically one of: active, idle, rx, tx. Regards, Tony From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752221Ab3FXKLE (ORCPT ); Mon, 24 Jun 2013 06:11:04 -0400 Received: from mho-03-ewr.mailhop.org ([204.13.248.66]:55672 "EHLO mho-01-ewr.mailhop.org" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751271Ab3FXKLC (ORCPT ); Mon, 24 Jun 2013 06:11:02 -0400 X-Mail-Handler: Dyn Standard SMTP by Dyn X-Originating-IP: 50.131.214.131 X-Report-Abuse-To: abuse@dyndns.com (see http://www.dyndns.com/services/sendlabs/outbound_abuse.html for abuse reporting information) X-MHO-User: U2FsdGVkX1+H83ffayisGjfrHuwnQEnx Date: Mon, 24 Jun 2013 03:10:55 -0700 From: Tony Lindgren To: Stephen Warren Cc: Linus Walleij , Linus Walleij , Stephen Warren , Kevin Hilman , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Ulf Hansson Subject: Re: [PATCH] pinctrl: document the pinctrl PM states Message-ID: <20130624101055.GT5523@atomide.com> References: <1370980749-15383-1-git-send-email-linus.walleij@stericsson.com> <51BA1FE7.4000900@wwwdotorg.org> <51BB3A2C.608@wwwdotorg.org> <20130617072021.GB4465@atomide.com> <51C20E3A.8090304@wwwdotorg.org> <20130620063823.GD5523@atomide.com> <51C3577E.6040709@wwwdotorg.org> <20130621062552.GI5523@atomide.com> <51C4A593.6000900@wwwdotorg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <51C4A593.6000900@wwwdotorg.org> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Stephen Warren [130621 12:18]: > On 06/21/2013 12:25 AM, Tony Lindgren wrote: > > * Stephen Warren [130620 12:32]: > >> > >> I assume you mean there shouldn't be any issue *modifying* the pinctrl > >> API to allow multiple states to be active at once? And where you're > >> talking about having multiple sets active at once already, you're > >> talking about some other API? > > > > Nope, the standard pinctrl API. At least I have not seen issues with > > having multiple states active the same time in a single driver. > > Please take a look at the implementation of pinctrl_select_state(). It > very explicitly performs the following steps: > > 1) Find all pins(groups) that are used in the current state but not the > new state, and execute pinctrl_disable_setting() on them. (For mux > settings only, not pin config, since the core doesn't have any idea how > to reverse config settings). > > 2) For all settings in the new state, apply those settings. > > So, it very explicitly only allows a single state to be set at a time. > Equally, p->state (the field which stores the currently selected state) > is a single item, not a set/list/array. OK thanks I get now what you're saying. I did not see the p->state issue as the disable function won't do anything for the SoCs that I mostly deal with. > So, this code needs rework if you want the core to support the concept > of having multiple states active at once, since it needs separate > pinctrl_activate_state() and pinctrl_deactivate_state() APIs, in order > to avoid step (1) above. And of course, p->state would need to be a > set/list/array. I'll think about it a bit and do a patch to fix this. It seems that that we need just two entries in the p->state array: static (default), and dynamic. Then the dynamic would be typically one of: active, idle, rx, tx. Regards, Tony