linux-kernel - RE: [PATCH V2 1/2] pinctrl: Introduce PINCTRL_STATE

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]

Date:	Wed, 29 Feb 2012 11:21:53 -0800
From:	Stephen Warren <swarren@...dia.com>
To:	Linus Walleij <linus.walleij@...aro.org>
CC:	Linus Walleij <linus.walleij@...ricsson.com>,
	"B29396@...escale.com" <B29396@...escale.com>,
	"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
	"dongas86@...il.com" <dongas86@...il.com>,
	"shawn.guo@...aro.org" <shawn.guo@...aro.org>,
	"thomas.abraham@...aro.org" <thomas.abraham@...aro.org>,
	"tony@...mide.com" <tony@...mide.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH V2 1/2] pinctrl: Introduce PINCTRL_STATE_DEFAULT, define
 hogs as that state

Stephen Warren wrote at Wednesday, February 29, 2012 10:42 AM:
> Linus Walleij wrote at Wednesday, February 29, 2012 9:41 AM:
> > Aha now I see the problem here...
> >
> > On Tue, Feb 28, 2012 at 12:55 AM, Stephen Warren <swarren@...dia.com> wrote:
> >
> > > This provides a single centralized name for the default state.
> > (...)
> > > -/* Hog a single map entry and add to the hoglist */
> > > -static int pinctrl_hog_map(struct pinctrl_dev *pctldev,
> > (...)
> > > -static int pinctrl_hog_maps(struct pinctrl_dev *pctldev)
> > (...)
> > > -       pinctrl_hog_maps(pctldev);
> > > +       pctldev->p = pinctrl_get(pctldev->dev, PINCTRL_STATE_DEFAULT);
> > > +       if (!IS_ERR(pctldev->p))
> > > +               pinctrl_enable(pctldev->p);
> >
> > So what happens here is that my hogs will try to activate three different
> > functions in the same struct pinctrl *p.
> >
> > This fails the sanity check in pinmux.c:
> >
> > 	/*
> > 	 * If the function selector is already set, it needs to be identical,
> > 	 * we support several groups with one function but not several
> > 	 * functions with one or several groups in the same pinmux.
> > 	 */
> > 	if (p->func_selector != UINT_MAX &&
> > 	    p->func_selector != func_selector) {
> > 		dev_err(pctldev->dev,
> > 			"dual function defines in the map for device %s\n",
> > 		       devname);
> > 		return -EINVAL;
> > 	}
> > 	p->func_selector = func_selector;
> 
> Oh right, I'd forgotten about that check at this stage in the patch
> series.
> 
> Later in the series, this restriction is removed; each setting in a state
> is completely independent, and could easily program a different function
> onto each group, or the same function. At this stage in the series, a
> struct pinctrl can't represent everything that a set of mapping table
> entries can, but later struct pinctrl is fully capable.
> 
> I guess the solution here is to just make this patch introduce the
> PINCTRL_STATE_DEFAULT define, but defer most usage of it until later
> in the series when this restriction is removed.

Actually, it looks very easy to move the func_selector field out of
struct pinctrl into struct pinmux_group, which would solve this problem
in what I consider the correct way. I'll code up a patch to do that and
stack it before this current patch.

-- 
nvpublic

--
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/