linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Linux-OMAP <linux-omap@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Stephen Warren <swarren@wwwdotorg.org>
Subject: Re: [PATCH 3/4] pinctrl: Add support for additional dynamic states
Date: Mon, 29 Jul 2013 02:31:25 -0700	[thread overview]
Message-ID: <20130729093125.GB7656@atomide.com> (raw)
In-Reply-To: <CACRpkdY6-f1DKfPDfCpYAX9weXMtB7koJ3VwkRgHGm_MzgFawQ@mail.gmail.com>

* Linus Walleij <linus.walleij@linaro.org> [130722 16:14]:
> On Tue, Jul 16, 2013 at 11:05 AM, Tony Lindgren <tony@atomide.com> 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.
> 
> OK...
> 
> > 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.
> 
> I do not understand why this complexity need to be exposed outside
> of the subsystem.

Unfortunately it's mostly to deal with supporting the current behaviour
of pinctrl_select_state() which is not quite suitable for runtime PM.
 
> pinctrl_select_state() and the PM accessors are enough IMO. Why
> should say a driver care about whether it is dynamic or not?

I think we can make this all transparent to the consumer drivers
for runtime PM. Basically drivers/base/pinctrl.c needs these for the
checks because of the current pinctrl_select_state().
 
> Surely the checking and different paths for static/dynamic configurations
> can be an intrinsic detail of the pinctrl subsystem, by adding flags and
> members to private structs like struct pinctrl itself in worst case.

I'll take a look if we can bury more things inside the pinctrl
subsystem.
 
> So I'm not buying into this, it looks like it is making things complicated
> for consumers outside the subsystem for no reason.

I don't think the consumer drivers eventually need to do much
anything ideally. We're missing runtime PM related set_irq_wake()
but that's a minor detail as we can initially keep the runtime
PM related wake-up events always enabled.

Regards,

Tony

  reply	other threads:[~2013-07-29  9:31 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
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 [this message]
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=20130729093125.GB7656@atomide.com \
    --to=tony@atomide.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=swarren@wwwdotorg.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).