[U-Boot] [PATCH V5 2/4] serial: Add Tegra2 serial port support

[Tom Warren]

Mike,

On Tue, Jan 25, 2011 at 1:11 AM, Mike Rapoport <mike at compulab.co.il> wrote:
> On 01/22/11 01:06, Tom Warren wrote:
>> Signed-off-by: Tom Warren <twarren at nvidia.com>
>> ---
>
> [ snip ]
>
>> +/*
>> + * Routine: pin_mux_uart
>> + * Description: setup the pin muxes/tristate values for a UART
>> + */
>> +static void pin_mux_uart(void)
>> +{
>> +     pinmux_tri_ctlr *const pmt = (pinmux_tri_ctlr *)NV_PA_APB_MISC_BASE;
>> +     u32 reg;
>> +
>> +#if CONFIG_TEGRA2_ENABLE_UARTA
>> +             reg = readl(&pmt->pmt_ctl_c);
>> +             reg &= 0xFFF0FFFF;      /* IRRX_/IRTX_SEL [19:16] = 00 UARTA */
>> +             writel(reg, &pmt->pmt_ctl_c);
>> +
>> +             reg = readl(&pmt->pmt_tri_a);
>> +             reg &= ~Z_IRRX;         /* Z_IRRX = normal (0) */
>> +             reg &= ~Z_IRTX;         /* Z_IRTX = normal (0) */
>> +             writel(reg, &pmt->pmt_tri_a);
>> +#endif       /* CONFIG_TEGRA2_ENABLE_UARTA */
>> +#if CONFIG_TEGRA2_ENABLE_UARTD
>> +             reg = readl(&pmt->pmt_ctl_b);
>> +             reg &= 0xFFFFFFF3;      /* GMC_SEL [3:2] = 00, UARTD */
>> +             writel(reg, &pmt->pmt_ctl_b);
>> +
>> +             reg = readl(&pmt->pmt_tri_a);
>> +             reg &= ~Z_GMC;          /* Z_GMC = normal (0) */
>> +             writel(reg, &pmt->pmt_tri_a);
>> +#endif       /* CONFIG_TEGRA2_ENABLE_UARTD */
>
> As I've already pointed out (1) this covers only one possibility of UART pin
> muxing options. I agree that it makes sense when this is a part of the
> board-specific code. However, forcing specific pins configuration in the generic
> driver seems problematic to me, even if most Tegra2 boards use the same
> configuration.
> As a side note, I'd prefer to have all the pin multiplexing defined in one place
> in board file rather than scattered among different drivers.
>
Alright. I'd like to get this wrapped up and accepted before the merge window
closes so I can get on with the important bits (drivers, etc.). What
would you like
here? A generic function like 'pinmux_set_config(reg, val, bit)' that
lives in the board
files and gets called from the driver code with specifics of that
board's/drivers pinmux
config? Or do you see the pinmux code living entirely in the board
files, and being done
as part of board init? That's where it was before, BTW.

If there are examples that you approve of in any extant U-Boot code (omap,
ti91, davinci, etc.), please point them out.  I'd really like to
finalize this patch series.

> (1) http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/92887/focus=93233
>
>
> --
> Sincerely yours,
> Mike.
Thanks,

Tom
>