Re: [PATCH v2 10/18] pinctrl: madera: Add driver for Cirrus Logic Madera codecs

From: Richard Fitzgerald
Date: Tue Apr 25 2017 - 06:26:38 EST


On Tue, 2017-04-25 at 11:41 +0200, Linus Walleij wrote:
> On Mon, Apr 24, 2017 at 6:08 PM, Richard Fitzgerald
> <rf@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> > These codecs have a variable number of I/O lines each of which
> > is individually selectable to a wide range of possible functions.
> >
> > The functionality is slightly different from the traditional muxed
> > GPIO since most of the functions can be mapped to any pin (and even
> > the same function to multiple pins). Most pins have a dedicated
> > "alternate" function that is only available on that pin. The
> > alternate functions are usually a group of signals, though it is
> > not always necessary to enable the full group, depending on the
> > alternate function and how it is to be used. The mapping between
> > alternate functions and GPIO pins varies between codecs depending
> > on the number of alternate functions and available pins.
> >
> > Note on the Kconfig options:
> > The formula "default y if..." is used for PINCTRL_MADERA so that its
> > select options will be processed, allowing us to group selects for
> > pinctrl into the pinctrl Kconfig where they logically belong instead
> > of accumulating under the parent MFD Kconfig.
> >
> > Signed-off-by: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
> (...)
>
> > Changes since V1:
>
> This is starting to look good!
>
> In fact, kind of finished. Except:
>
> > + if (pdata) {
> > + ret = pinctrl_register_mappings(pdata->gpio_configs,
> > + pdata->n_gpio_configs);
> > + if (ret) {
> > + dev_err(priv->dev,
> > + "Failed to register pdata mappings (%d)\n",
> > + ret);
> > + return ret;
> > + }
> > + }
>
> What is this? It looks like boardfile support which we do not like to
> encourage. Why are the mappings not exclusively set up from
> device tree or similar?
>

We want to work on systems that don't use DT for configuration, so we
offer pdata as a fallback across all the drivers. In that case the
configuration is defined as part of the parent pdata struct and this
applies it.

> Yours,
> Linus Walleij