[U-Boot] [PATCH 3/6] tegra: Add a function mux feature

[Stephen Warren <swarren@nvidia.com>]

On 11/23/2011 03:59 PM, Simon Glass wrote:
> funcmux permits selection of config options for particular peripherals,
> such as the pins that are used for that peripheral, if there are several
> options.
> 
> Add UART selection to start with.

> +static void enable_uart(enum periph_id pid)
> +{
> +	/* Assert UART reset and enable clock */
> +	reset_set_enable(pid, 1);
> +	clock_enable(pid);
> +	clock_ll_set_source(pid, 0);	/* UARTx_CLK_SRC = 00, PLLP_OUT0 */
> +
> +	/* wait for 2us */
> +	udelay(2);
> +
> +	/* De-assert reset to UART */
> +	reset_set_enable(pid, 0);
> +}

That doesn't seem like anything to do with function muxing.

> +void funcmux_select(enum periph_id id, int func)

Parameter "func" doesn't appear to be used. Is it to support e.g. UART1
being routed to different sets of pins based on board design? If so, the
values of "func" should be defined by this patch too, and validated in
the code below, so that people don't start passing bogus data without
issue now, then suddenly get hit when we see boards with different
pinmux configurations.

> +{
> +	switch (id) {
> +	case PERIPH_ID_UART1:
> +		pinmux_set_func(PINGRP_IRRX, PMUX_FUNC_UARTA);
> +		pinmux_set_func(PINGRP_IRTX, PMUX_FUNC_UARTA);
> +		pinmux_tristate_disable(PINGRP_IRRX);
> +		pinmux_tristate_disable(PINGRP_IRTX);
> +		break;
> +
> +	case PERIPH_ID_UART2:
> +		pinmux_set_func(PINGRP_UAD, PMUX_FUNC_IRDA);
> +		pinmux_tristate_disable(PINGRP_UAD);
> +		break;
> +
> +	case PERIPH_ID_UART4:
> +		pinmux_set_func(PINGRP_GMC, PMUX_FUNC_UARTD);
> +		pinmux_tristate_disable(PINGRP_GMC);
> +		break;
> +
> +	default:
> +		debug("%s: invalid periph_id %d", __func__, id);
> +		break;
> +	}
> +}

I'm not entirely convinced that centralizing this in a function rather
than putting the board-specific muxing into the per-board files is the
right way to go. What's wrong with the kernel's approach of a single
table describing each board's complete pinmux settings? Eventually, all
this will come from DT anyway, won't it?

-- 
nvpublic