RE: [PATCH V2 1/2] pinctrl: Introduce PINCTRL_STATE_DEFAULT, define hogs as that state

[Stephen Warren <swarren@nvidia.com>]

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@nvidia.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