RE: [PATCH 2/2 v4] pinctrl: introduce generic pin config

[Stephen Warren <swarren@nvidia.com>]

Linus Walleij wrote at Thursday, December 01, 2011 3:12 AM:
> On Wed, Nov 30, 2011 at 8:45 PM, Stephen Warren <swarren@nvidia.com> wrote:
> > Linus Walleij wrote at Thursday, November 24, 2011 11:46 AM:
> 
> >> diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
> >
> >> +/*
> >> + * You shouldn't even be able to compile with these enums etc unless you're
> >> + * using generic pin config. That is why this is defined out.
> >> + */
> >> +#ifdef CONFIG_GENERIC_PINCONF
> >
> > Hmm. I'd prefer to have drivers able to use both generic values and
> > extend them with custom values. Can't we just use the top bit of the
> > param value to indicate 0:standard (from the enum below) 1:custom
> > (something meaningful to only the individual pinctrl driver). This
> > could then trigger calling pinconf_generic_dump_pin() or not for
> > example.
> 
> Hm...
> 
> That will get messy when if I refactor this stuff, add new enums
> and whatever.

I don't understand what'd be difficult about that.

New standardized enums could be added with values without the top bit
set. No existing driver would need modification, since their switch(param)
would not have a case for that new value, and would just return an error.

> >> +#ifdef CONFIG_GENERIC_PINCONF
> >> +     bool is_generic;
> >> +#endif
> >
> > ... and get rid of that flag.
> 
> This is for the case wher you have both generic and non-generic
> config controllers onboard a system.
> 
> Like in the generic debugfs dump function:
> 
> 	if (!ops->is_generic)
> 		return;
> 
> If I take this out, the generic debugfs code will be used for
> everything.

I think that'd be fine; the generic code would do all the debug prints
for the standardized enums, then the core would call into the pinctrl
driver to perform any additional debug prints for any driver-defined
custom parameters.

> And then the generic sematics which you didn't like in the
> previous patch:
> 
> if (ret == -EINVAL || ret == -ENOTSUPP)
> 
> Need to go back in, else the generic debugfs stuff won't
> work.

That'd be fine, provided the loop only checked the standardized parameters,
or only enabled that special error-checking case for standardized parameters.

-- 
nvpublic