From: swarren@wwwdotorg•org (Stephen Warren)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 3/4] pinctrl: Add support for additional dynamic states
Date: Thu, 18 Jul 2013 13:21:02 -0600 [thread overview]
Message-ID: <51E8401E.8040705@wwwdotorg.org> (raw)
In-Reply-To: <20130718072034.GO7656@atomide.com>
On 07/18/2013 01:25 AM, Tony Lindgren wrote:
> * Stephen Warren <swarren@wwwdotorg•org> [130717 14:21]:
>> On 07/16/2013 03:05 AM, Tony Lindgren wrote:
>>> To toggle dynamic states, let's add the optional active state in
>>> addition to the static default state. Then if the optional active
>>> state is defined, we can require that idle and sleep states cover
>>> the same pingroups as the active state.
>>>
>>> Then let's add pinctrl_check_dynamic() and pinctrl_select_dynamic()
>>> to use instead of pinctrl_select() to avoid breaking existing users.
>>>
>>> With pinctrl_check_dynamic() we can check that idle and sleep states
>>> match the active state for pingroups during init, and don't need to
>>> do it during runtime.
>>>
>>> Then with the states pre-validated, pinctrl_select_dynamic() can
>>> just toggle between the dynamic states without extra checks.
>>>
>>> Note that pinctr_select_state() still has valid use cases, such as
>>> changing states when the pins can be shared between two drivers
>>> and don't necessarily cover the same pingroups. For dynamic runtime
>>> toggling of pin states, we should eventually always use just
>>> pinctrl_select_dynamic().
>>> @@ -1241,7 +1371,13 @@ static int pinctrl_pm_select_state(struct device *dev, struct pinctrl_state *sta
>>> return 0;
>>> if (IS_ERR(state))
>>> return 0; /* No such state */
>>> - ret = pinctrl_select_state(pins->p, state);
>>> +
>>> + /* Configured for proper dynamic muxing? */
>>> + if (!IS_ERR(dev->pins->active_state))
>>> + ret = pinctrl_select_dynamic(pins->p, state);
>>> + else
>>> + ret = pinctrl_select_state(pins->p, state);
>>
>> Hmmm. I'm not quite sure this is right... Surely this function should
>> just do nothing if the dynamic states aren't defined? The system should
>> just, and only, be in the "default" state and never do anything else.
>
> This keeps the existing behaviour. We should be able to drop the
> call to pinctrl_select_state() here after the existing use in
> drivers has been fixed.
How many DT-based systems already have multiple of default/idle/sleep
states defined in DT? Right now, since the kernel code uses
pinctrl_select_state() to switch between those, the state definitions
need to define /all/ pins used by those states, not just the dynamic
ones. However, with this series in place, the default state should only
include the static pins, and the active/idle/sleep states should only
include the dynamic pins. That's a change to the DT bindings, since it
changes how the DT must be written, and causes older DTs to be
incompatible with newer kernels and vice-versa.
So, do we just ignore this DT ABI change again, or insist on doing it in
some compatible way? DT ABI-ness is a PITA:-(
next prev parent reply other threads:[~2013-07-18 19:21 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-16 9:05 [PATCH 0/4] improved support for runtime muxing for pinctrl Tony Lindgren
2013-07-16 9:05 ` [PATCH 1/4] pinctrl: Remove duplicate code in pinctrl_pm_select_state functions Tony Lindgren
2013-07-16 13:15 ` Grygorii Strashko
2013-07-16 13:41 ` Tony Lindgren
2013-07-16 14:25 ` Grygorii Strashko
2013-07-17 6:31 ` Tony Lindgren
2013-07-16 9:05 ` [PATCH 2/4] pinctrl: Allow pinctrl to have multiple active states Tony Lindgren
2013-07-17 20:55 ` Stephen Warren
2013-07-16 9:05 ` [PATCH 3/4] pinctrl: Add support for additional dynamic states Tony Lindgren
2013-07-16 9:35 ` Felipe Balbi
2013-07-16 12:06 ` Tony Lindgren
2013-07-17 21:14 ` Stephen Warren
2013-07-18 7:25 ` Tony Lindgren
2013-07-18 10:53 ` Tony Lindgren
2013-07-18 19:21 ` Stephen Warren [this message]
2013-07-19 7:29 ` Tony Lindgren
2013-07-19 18:52 ` Stephen Warren
2013-07-29 9:05 ` Tony Lindgren
2013-07-29 22:01 ` Stephen Warren
2013-08-14 16:41 ` Linus Walleij
2013-07-17 21:23 ` Stephen Warren
2013-07-18 7:36 ` Tony Lindgren
2013-07-18 19:26 ` Stephen Warren
2013-07-19 7:39 ` Tony Lindgren
2013-07-19 10:29 ` Grygorii Strashko
2013-07-19 19:03 ` Stephen Warren
2013-07-22 23:15 ` Linus Walleij
2013-07-29 9:08 ` Tony Lindgren
2013-07-19 18:58 ` Stephen Warren
2013-07-29 9:21 ` Tony Lindgren
2013-07-29 22:08 ` Stephen Warren
2013-07-22 23:07 ` Linus Walleij
2013-07-29 9:31 ` Tony Lindgren
2013-07-16 9:05 ` [PATCH 4/4] drivers: Add pinctrl handling for dynamic pin states Tony Lindgren
2013-07-17 21:21 ` Stephen Warren
2013-07-18 7:50 ` Tony Lindgren
2013-07-18 13:48 ` Tony Lindgren
2013-07-16 9:14 ` [PATCH 0/4] improved support for runtime muxing for pinctrl Tony Lindgren
2013-07-17 11:49 ` Grygorii Strashko
-- strict thread matches above, loose matches on Subject: below --
2013-07-18 15:15 [PATCHv2 " Tony Lindgren
2013-07-18 15:15 ` [PATCH 3/4] pinctrl: Add support for additional dynamic states Tony Lindgren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51E8401E.8040705@wwwdotorg.org \
--to=swarren@wwwdotorg$(echo .)org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox