From: tony@atomide.com (Tony Lindgren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] drivers: pinctrl: add active state to core
Date: Mon, 17 Jun 2013 00:29:52 -0700 [thread overview]
Message-ID: <20130617072951.GC4465@atomide.com> (raw)
In-Reply-To: <51BB357F.80406@wwwdotorg.org>
* Stephen Warren <swarren@wwwdotorg.org> [130614 08:29]:
> On 06/14/2013 02:46 AM, Tony Lindgren wrote:
> >
> > Hmm how would the pinctrl subsystem know which pins to reprogram and
> > which ones are static?
>
> Each state should list the desired configuration of all pins used by the
> HW module. Any configuration that's identical between the old an new
> state when pinctrl_select_state() executes is static, anything else is not.
Listing all the pins in each named mode won't work too well from hardware
point of view, I think this can be solved by having a bit finer grained
named modes. I've just replied about this in the related thread
"[PATCH] pinctrl: document the pinctrl PM states".
> > We don't have any default state for pins for omaps at least and pinctrl
> > single does not not set them to anything when disable is called unless
> > function-off is specified.
>
> But that's OMAP-specific. If the set of pinctrl states that the core PM
> code operates on is documented as general policy, it has to work the
> same everywhere.
Agreed. Hopefully this issue too is addressed in the other reply I
mention above.
> > But even if the pinctrl driver does something to the pins in disable,
> > that should work too. It's just an extra step between toggling between
> > two named pin states.
>
> If the "default" state says e.g. "set pin X to function A", and the
> "idle" state doesn't say anything about pin X, when a switch from
> default -> idle will simply program pin X back to its default state (if
> the driver does that) and then not program it to anything else, since
> the idle state doesn't say what to program it to.
>
> As I said, this might work fine if the pinctrl driver doesn't do
> anything in struct pinmux_ops .disable, but given that it's legal for
> the driver to do so, relying on that won't work for all drivers, so some
> alternative scheme of handling static pinmux configuration is needed.
And hopefully this issue too is addressed :)
> >> Also, if default uses more pins that active, when you switch to active,
> >> those pins won't be marked as owned any more, I think, so something else
> >> could in theory grab them. At least, debugfs wouldn't be entirely
> >> accurate any more.
> >
> > I think you're misunderstanding. The default pins are held for as long
> > as the device is loaded. We're not touching the default pins at all
> > after probe. Active and idle pins are not subsets of default.
>
> OK, then please provide an example of how this is represented.
I've listed a few examples in the other thread, so maybe take a look
and see if it addresses your concerns.
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>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Stephen Warren <swarren@nvidia.com>,
Kevin Hilman <khilman@linaro.org>,
Wolfram Sang <wsa@the-dreams.de>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Hebbar Gururaja <gururaja.hebbar@ti.com>,
Mark Brown <broonie@kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
Ulf Hansson <ulf.hansson@linaro.org>
Subject: Re: [PATCH] drivers: pinctrl: add active state to core
Date: Mon, 17 Jun 2013 00:29:52 -0700 [thread overview]
Message-ID: <20130617072951.GC4465@atomide.com> (raw)
In-Reply-To: <51BB357F.80406@wwwdotorg.org>
* Stephen Warren <swarren@wwwdotorg.org> [130614 08:29]:
> On 06/14/2013 02:46 AM, Tony Lindgren wrote:
> >
> > Hmm how would the pinctrl subsystem know which pins to reprogram and
> > which ones are static?
>
> Each state should list the desired configuration of all pins used by the
> HW module. Any configuration that's identical between the old an new
> state when pinctrl_select_state() executes is static, anything else is not.
Listing all the pins in each named mode won't work too well from hardware
point of view, I think this can be solved by having a bit finer grained
named modes. I've just replied about this in the related thread
"[PATCH] pinctrl: document the pinctrl PM states".
> > We don't have any default state for pins for omaps at least and pinctrl
> > single does not not set them to anything when disable is called unless
> > function-off is specified.
>
> But that's OMAP-specific. If the set of pinctrl states that the core PM
> code operates on is documented as general policy, it has to work the
> same everywhere.
Agreed. Hopefully this issue too is addressed in the other reply I
mention above.
> > But even if the pinctrl driver does something to the pins in disable,
> > that should work too. It's just an extra step between toggling between
> > two named pin states.
>
> If the "default" state says e.g. "set pin X to function A", and the
> "idle" state doesn't say anything about pin X, when a switch from
> default -> idle will simply program pin X back to its default state (if
> the driver does that) and then not program it to anything else, since
> the idle state doesn't say what to program it to.
>
> As I said, this might work fine if the pinctrl driver doesn't do
> anything in struct pinmux_ops .disable, but given that it's legal for
> the driver to do so, relying on that won't work for all drivers, so some
> alternative scheme of handling static pinmux configuration is needed.
And hopefully this issue too is addressed :)
> >> Also, if default uses more pins that active, when you switch to active,
> >> those pins won't be marked as owned any more, I think, so something else
> >> could in theory grab them. At least, debugfs wouldn't be entirely
> >> accurate any more.
> >
> > I think you're misunderstanding. The default pins are held for as long
> > as the device is loaded. We're not touching the default pins at all
> > after probe. Active and idle pins are not subsets of default.
>
> OK, then please provide an example of how this is represented.
I've listed a few examples in the other thread, so maybe take a look
and see if it addresses your concerns.
Regards,
Tony
next prev parent reply other threads:[~2013-06-17 7:29 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-11 8:16 [PATCH] drivers: pinctrl: add active state to core Linus Walleij
2013-06-11 8:16 ` Linus Walleij
2013-06-11 16:03 ` Greg Kroah-Hartman
2013-06-11 16:03 ` Greg Kroah-Hartman
2013-06-12 18:35 ` Tony Lindgren
2013-06-12 18:35 ` Tony Lindgren
2013-06-11 16:33 ` Stephen Warren
2013-06-11 16:33 ` Stephen Warren
2013-06-11 19:53 ` Linus Walleij
2013-06-11 19:53 ` Linus Walleij
2013-06-12 18:33 ` Tony Lindgren
2013-06-12 18:33 ` Tony Lindgren
2013-06-13 19:31 ` Stephen Warren
2013-06-13 19:31 ` Stephen Warren
2013-06-14 8:46 ` Tony Lindgren
2013-06-14 8:46 ` Tony Lindgren
2013-06-14 15:23 ` Stephen Warren
2013-06-14 15:23 ` Stephen Warren
2013-06-17 7:29 ` Tony Lindgren [this message]
2013-06-17 7:29 ` Tony Lindgren
2013-06-13 19:38 ` Stephen Warren
2013-06-13 19:38 ` Stephen Warren
2013-06-16 10:01 ` Linus Walleij
2013-06-16 10:01 ` Linus Walleij
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=20130617072951.GC4465@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.