[U-Boot] [PATCH 3/5] vybrid: add uart driver support

[Stefano Babic]

On 12/04/2013 08:53, Alison Wang wrote:
> This patch adds uart driver support for vybrid platform.
> 
> Signed-off-by: TsiChung Liew <tsicliew at gmail.com>
> Signed-off-by: Jason Jin <Jason.jin at freescale.com>
> Signed-off-by: Alison Wang <b18965 at freescale.com>
> ---

Hi Alison

> diff --git a/drivers/serial/serial_vybrid.c b/drivers/serial/serial_vybrid.c
> new file mode 100644
> index 0000000..4dd9b52
> --- /dev/null
> +++ b/drivers/serial/serial_vybrid.c
> @@ -0,0 +1,129 @@
> +/*
> + * Copyright 2012-2013 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + */
> +
> +#include <common.h>
> +#include <watchdog.h>
> +#include <asm/io.h>
> +#include <serial.h>
> +#include <linux/compiler.h>
> +#include <asm/arch/vybrid-regs.h>
> +#include <asm/arch/serial-vybrid.h>
> +#include <asm/arch/clock.h>
> +
> +#ifndef CONFIG_VYBRID_UART_BASE
> +#error "define CONFIG_VYBRID_UART_BASE to use the VYBRID UART driver"
> +#endif
> +
> +#define UART_CONSOLE	\
> +		(CONFIG_VYBRID_UART_BASE + (CONFIG_SYS_UART_PORT * 0x1000))
> +
> +#ifdef CONFIG_SERIAL_MULTI
> +#warning "Vybrid driver does not support MULTI serials."
> +#endif
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static void vybrid_serial_setbrg(void)
> +{
> +	u32 clk = vybrid_get_uartclk();
> +	u16 sbr;
> +
> +	if (!gd->baudrate)
> +		gd->baudrate = CONFIG_BAUDRATE;
> +
> +	sbr = (u16)(clk / (16 * gd->baudrate));
> +	/* place adjustment later - n/32 BRFA */
> +
> +	out_8((UART_CONSOLE + UBDH), (sbr >> 8));
> +	out_8((UART_CONSOLE + UBDL), (sbr & 0xFF));
> +}
> +
> +static int vybrid_serial_getc(void)
> +{
> +	while (!(in_8(UART_CONSOLE + US1) & US1_RDRF))
> +		WATCHDOG_RESET();
> +

Generally : do not use BASE + offset as here. Use C structures, instead.
We have already ARM accessors (in8 is defined for powerpc). Use
functions in io.h

This driver is very simple, and it is similar to already implemented
drivers. Sure that we cannot reuse other code ?

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================