From: tony@atomide.com (Tony Lindgren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] pinctrl: document the pinctrl PM states
Date: Mon, 24 Jun 2013 03:10:55 -0700 [thread overview]
Message-ID: <20130624101055.GT5523@atomide.com> (raw)
In-Reply-To: <51C4A593.6000900@wwwdotorg.org>
* Stephen Warren <swarren@wwwdotorg.org> [130621 12:18]:
> On 06/21/2013 12:25 AM, Tony Lindgren wrote:
> > * Stephen Warren <swarren@wwwdotorg.org> [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
WARNING: multiple messages have this Message-ID (diff)
From: Tony Lindgren <tony@atomide.com>
To: Stephen Warren <swarren@wwwdotorg.org>
Cc: Linus Walleij <linus.walleij@linaro.org>,
Linus Walleij <linus.walleij@stericsson.com>,
Stephen Warren <swarren@nvidia.com>,
Kevin Hilman <khilman@linaro.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Ulf Hansson <ulf.hansson@linaro.org>
Subject: Re: [PATCH] pinctrl: document the pinctrl PM states
Date: Mon, 24 Jun 2013 03:10:55 -0700 [thread overview]
Message-ID: <20130624101055.GT5523@atomide.com> (raw)
In-Reply-To: <51C4A593.6000900@wwwdotorg.org>
* Stephen Warren <swarren@wwwdotorg.org> [130621 12:18]:
> On 06/21/2013 12:25 AM, Tony Lindgren wrote:
> > * Stephen Warren <swarren@wwwdotorg.org> [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
next prev parent reply other threads:[~2013-06-24 10:10 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-11 19:59 [PATCH] pinctrl: document the pinctrl PM states Linus Walleij
2013-06-11 19:59 ` Linus Walleij
2013-06-12 18:37 ` Tony Lindgren
2013-06-12 18:37 ` Tony Lindgren
2013-06-13 19:39 ` Stephen Warren
2013-06-13 19:39 ` Stephen Warren
2013-06-13 20:34 ` Linus Walleij
2013-06-13 20:34 ` Linus Walleij
2013-06-14 15:43 ` Stephen Warren
2013-06-14 15:43 ` Stephen Warren
2013-06-16 10:17 ` Linus Walleij
2013-06-16 10:17 ` Linus Walleij
2013-06-17 7:20 ` Tony Lindgren
2013-06-17 7:20 ` Tony Lindgren
2013-06-17 15:56 ` Linus Walleij
2013-06-17 15:56 ` Linus Walleij
2013-06-17 18:06 ` Tony Lindgren
2013-06-17 18:06 ` Tony Lindgren
2013-06-17 18:15 ` Rohit Vaswani
2013-06-17 18:15 ` Rohit Vaswani
2013-06-17 16:05 ` Linus Walleij
2013-06-17 16:05 ` Linus Walleij
2013-06-17 18:02 ` Tony Lindgren
2013-06-17 18:02 ` Tony Lindgren
2013-06-19 20:06 ` Stephen Warren
2013-06-19 20:06 ` Stephen Warren
2013-06-24 12:37 ` Linus Walleij
2013-06-24 12:37 ` Linus Walleij
2013-06-25 7:31 ` Tony Lindgren
2013-06-25 7:31 ` Tony Lindgren
2013-06-19 20:02 ` Stephen Warren
2013-06-19 20:02 ` Stephen Warren
2013-06-20 6:38 ` Tony Lindgren
2013-06-20 6:38 ` Tony Lindgren
2013-06-20 19:26 ` Stephen Warren
2013-06-20 19:26 ` Stephen Warren
2013-06-21 6:25 ` Tony Lindgren
2013-06-21 6:25 ` Tony Lindgren
2013-06-21 19:12 ` Stephen Warren
2013-06-21 19:12 ` Stephen Warren
2013-06-24 10:10 ` Tony Lindgren [this message]
2013-06-24 10:10 ` Tony Lindgren
2013-06-24 18:09 ` Stephen Warren
2013-06-24 18:09 ` Stephen Warren
2013-06-25 7:38 ` Tony Lindgren
2013-06-25 7:38 ` 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=20130624101055.GT5523@atomide.com \
--to=tony@atomide.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.