linux-kernel - Re: [PATCH 3/4] pinctrl: Add support for additional dynamic states

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130718105345.GV7656@atomide.com>
Date:	Thu, 18 Jul 2013 03:53:46 -0700
From:	Tony Lindgren <tony@...mide.com>
To:	Stephen Warren <swarren@...dotorg.org>
Cc:	linus.walleij@...aro.org, linux-omap@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 3/4] pinctrl: Add support for additional dynamic states

* Tony Lindgren <tony@...mide.com> [130718 00:31]:
> * Stephen Warren <swarren@...dotorg.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/