From: Tony Lindgren <tony@atomide.com> To: Stephen Warren <swarren@wwwdotorg.org> Cc: linus.walleij@linaro.org, linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 3/4] pinctrl: Add support for additional dynamic states Date: Thu, 18 Jul 2013 03:53:46 -0700 [thread overview] Message-ID: <20130718105345.GV7656@atomide.com> (raw) In-Reply-To: <20130718072034.GO7656@atomide.com> * Tony Lindgren <tony@atomide.com> [130718 00:31]: > * Stephen Warren <swarren@wwwdotorg.org> [130717 14:21]: > > On 07/16/2013 03:05 AM, Tony Lindgren wrote: > > > +int pinctrl_check_dynamic(struct device *dev, struct pinctrl_state *st1, > > > + struct pinctrl_state *st2) > > > +{ > > > + struct pinctrl_setting *s1, *s2; > > > + > > > + list_for_each_entry(s1, &st1->settings, node) { > > ... > > > + list_for_each_entry(s2, &st2->settings, node) { > > ... > > > + if (pctldev1 != pctldev2) { > > > + dev_dbg(dev, "pctldev must be the same for states\n"); > > > + return -EINVAL; > > > + } > > > > I don't think we should require that; it's perfectly legal at the moment > > for some device's pinctrl configuration to include settings from > > multiple different pin controllers. > > Yes that's fine for pinctrl_select(), but let's not do that for > runtime muxing. Hmm reading this again, you're right, there should not be anything preventing mixing multiple controllers as long as the states match. > > > + for (i = 0; i < num_pins1; i++) { > > > + int pin1 = pins1[i]; > > > + > > > + for (j = 0; j < num_pins2; j++) { > > > + int pin2 = pins2[j]; > > > + > > > + if (pin1 == pin2) { > > > + found++; > > > + break; > > > + } > > > + } > > > + } > > > + > > > + if (found != num_pins1) { > > > > I guess this make sure that every entry in the dynamic state is in the > > state state, but not vice-versa; the static state can affect more stuff > > than the dynamic state? > > > > If so, shouldn't that check be if (found != num_pins2)? > > The check is that idle_state and sleep_state pins must match the > active_state pins. This is intentionally different from the current > pinctrl_select() logic. And here things will get messed up if the order of settings is diffrent.. So yes, pinctrl_check_dynamic() needs more work. Regards, Tony
WARNING: multiple messages have this Message-ID (diff)
From: tony@atomide.com (Tony Lindgren) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 3/4] pinctrl: Add support for additional dynamic states Date: Thu, 18 Jul 2013 03:53:46 -0700 [thread overview] Message-ID: <20130718105345.GV7656@atomide.com> (raw) In-Reply-To: <20130718072034.GO7656@atomide.com> * Tony Lindgren <tony@atomide.com> [130718 00:31]: > * Stephen Warren <swarren@wwwdotorg.org> [130717 14:21]: > > On 07/16/2013 03:05 AM, Tony Lindgren wrote: > > > +int pinctrl_check_dynamic(struct device *dev, struct pinctrl_state *st1, > > > + struct pinctrl_state *st2) > > > +{ > > > + struct pinctrl_setting *s1, *s2; > > > + > > > + list_for_each_entry(s1, &st1->settings, node) { > > ... > > > + list_for_each_entry(s2, &st2->settings, node) { > > ... > > > + if (pctldev1 != pctldev2) { > > > + dev_dbg(dev, "pctldev must be the same for states\n"); > > > + return -EINVAL; > > > + } > > > > I don't think we should require that; it's perfectly legal at the moment > > for some device's pinctrl configuration to include settings from > > multiple different pin controllers. > > Yes that's fine for pinctrl_select(), but let's not do that for > runtime muxing. Hmm reading this again, you're right, there should not be anything preventing mixing multiple controllers as long as the states match. > > > + for (i = 0; i < num_pins1; i++) { > > > + int pin1 = pins1[i]; > > > + > > > + for (j = 0; j < num_pins2; j++) { > > > + int pin2 = pins2[j]; > > > + > > > + if (pin1 == pin2) { > > > + found++; > > > + break; > > > + } > > > + } > > > + } > > > + > > > + if (found != num_pins1) { > > > > I guess this make sure that every entry in the dynamic state is in the > > state state, but not vice-versa; the static state can affect more stuff > > than the dynamic state? > > > > If so, shouldn't that check be if (found != num_pins2)? > > The check is that idle_state and sleep_state pins must match the > active_state pins. This is intentionally different from the current > pinctrl_select() logic. And here things will get messed up if the order of settings is diffrent.. So yes, pinctrl_check_dynamic() needs more work. Regards, Tony
next prev parent reply other threads:[~2013-07-18 10:53 UTC|newest] Thread overview: 89+ 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 ` 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 9:05 ` Tony Lindgren 2013-07-16 13:15 ` Grygorii Strashko 2013-07-16 13:15 ` Grygorii Strashko 2013-07-16 13:15 ` Grygorii Strashko 2013-07-16 13:41 ` Tony Lindgren 2013-07-16 13:41 ` Tony Lindgren 2013-07-16 14:25 ` Grygorii Strashko 2013-07-16 14:25 ` Grygorii Strashko 2013-07-16 14:25 ` Grygorii Strashko 2013-07-17 6:31 ` Tony Lindgren 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-16 9:05 ` Tony Lindgren 2013-07-17 20:55 ` Stephen Warren 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:05 ` Tony Lindgren 2013-07-16 9:35 ` Felipe Balbi 2013-07-16 9:35 ` Felipe Balbi 2013-07-16 9:35 ` Felipe Balbi 2013-07-16 12:06 ` Tony Lindgren 2013-07-16 12:06 ` Tony Lindgren 2013-07-17 21:14 ` Stephen Warren 2013-07-17 21:14 ` Stephen Warren 2013-07-18 7:25 ` Tony Lindgren 2013-07-18 7:25 ` Tony Lindgren 2013-07-18 10:53 ` Tony Lindgren [this message] 2013-07-18 10:53 ` Tony Lindgren 2013-07-18 19:21 ` Stephen Warren 2013-07-18 19:21 ` Stephen Warren 2013-07-19 7:29 ` Tony Lindgren 2013-07-19 7:29 ` Tony Lindgren 2013-07-19 18:52 ` Stephen Warren 2013-07-19 18:52 ` Stephen Warren 2013-07-29 9:05 ` Tony Lindgren 2013-07-29 9:05 ` Tony Lindgren 2013-07-29 22:01 ` Stephen Warren 2013-07-29 22:01 ` Stephen Warren 2013-08-14 16:41 ` Linus Walleij 2013-08-14 16:41 ` Linus Walleij 2013-08-14 16:41 ` Linus Walleij 2013-07-17 21:23 ` Stephen Warren 2013-07-17 21:23 ` Stephen Warren 2013-07-18 7:36 ` Tony Lindgren 2013-07-18 7:36 ` Tony Lindgren 2013-07-18 19:26 ` Stephen Warren 2013-07-18 19:26 ` Stephen Warren 2013-07-19 7:39 ` Tony Lindgren 2013-07-19 7:39 ` Tony Lindgren 2013-07-19 10:29 ` Grygorii Strashko 2013-07-19 10:29 ` Grygorii Strashko 2013-07-19 10:29 ` Grygorii Strashko 2013-07-19 19:03 ` Stephen Warren 2013-07-19 19:03 ` Stephen Warren 2013-07-22 23:15 ` Linus Walleij 2013-07-22 23:15 ` Linus Walleij 2013-07-22 23:15 ` Linus Walleij 2013-07-29 9:08 ` Tony Lindgren 2013-07-29 9:08 ` Tony Lindgren 2013-07-19 18:58 ` Stephen Warren 2013-07-19 18:58 ` Stephen Warren 2013-07-29 9:21 ` Tony Lindgren 2013-07-29 9:21 ` Tony Lindgren 2013-07-29 22:08 ` Stephen Warren 2013-07-29 22:08 ` Stephen Warren 2013-07-22 23:07 ` Linus Walleij 2013-07-22 23:07 ` Linus Walleij 2013-07-22 23:07 ` Linus Walleij 2013-07-29 9:31 ` Tony Lindgren 2013-07-29 9:31 ` Tony Lindgren 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-16 9:05 ` Tony Lindgren 2013-07-17 21:21 ` Stephen Warren 2013-07-17 21:21 ` Stephen Warren 2013-07-18 7:50 ` Tony Lindgren 2013-07-18 7:50 ` Tony Lindgren 2013-07-18 13:48 ` 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-16 9:14 ` Tony Lindgren 2013-07-17 11:49 ` Grygorii Strashko 2013-07-17 11:49 ` Grygorii Strashko 2013-07-17 11:49 ` Grygorii Strashko 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 2013-07-18 15:15 ` 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=20130718105345.GV7656@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: linkBe 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.