[U-Boot] [PATCH v2 06/19] tegra: Add display support to funcmux

Simon Glass
2012-07-11T03:48:46Z

Hi Stephen,

On Fri, Jun 15, 2012 at 1:24 AM, Stephen Warren <swarren@wwwdotorg.org>wrote:

> On 06/13/2012 10:19 AM, Simon Glass wrote:
> > Add support for a default pin mapping for display1.
>
> > +static const struct pingroup_config disp1_default[] = {
> > + PINMUX(LM0, RSVD4, NORMAL, NORMAL),
> > + PINMUX(LPW1, RSVD4, NORMAL, TRISTATE),
>
> Do you really need to explicitly program these pingroups that aren't
> used by display? It seems like it'll probably still work fine
> irrespective of whether those are actually configured to output display
> controller signals or not. However, if those pingroups are used by
> something else, then changing their configuration here may stop the
> other functionality from working.
>
> > + PINMUX(SLXD, SPDIF, NORMAL, NORMAL),
>
> Similarly here, yet SLXD doesn't ever support any display functionality,
> so there should definitely be no need for this entry.
>

I will remove LPW1, but if I remove any others the LCD stops working.

>
> > @@ -185,6 +210,20 @@ int funcmux_select(enum periph_id id, int config)
>
> > + case PERIPH_ID_DISP1:
> > + if (config == FUNCMUX_DEFAULT) {
>
> Shouldn't this option have some kind of name in funcmux.h; there are
> many many many possible configurations for display, so just assuming
> this as some kind of default seems a little presumptuous.
>

It could have a name, but what would we call it? Until there is a second
option implemented, it seems reasonable to call it the default. That is
what was done with keyboard.

>
> But, given the number of possibilities, I wonder if it isn't time to
> replace funcmux with the device tree pinctrl bindings?
>

I suggest we look at this after we get LCD, NAND and all the other pending
things in. I haven't seen an fdt binding for Tegra LCD in the kernel yet,
so our binding will need looking at as well when it eventually happens.

Regards,
Simon