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

[Mike Rapoport]

On 01/25/11 18:50, Tom Warren wrote:
> 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.

I think that the pinmux code should live entirely in the board file and
performed as part of board init. The drivers than can assume that the pins are
configured properly.

> 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.

I think OMAP is a good example. There's set_muxconf_regs function in each board
file that is responsible for configuration of all the pins.

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


-- 
Sincerely yours,
Mike.