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

[Simon Glass]

Hi Stephen,

On Fri, Jun 15, 2012 at 1:24 AM, Stephen Warren <swarren at 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