[U-Boot] [RFC][PATCH v2] ARM: mxs: Added application UART driver

[Marek Vasut <marex@denx.de>]

Dear Andreas Wass,

> The driver is ported from a driver that was implemented
> using u-boot 2009.

What driver? Anyway, this doesn't go here. If it's ported from LTIB, stick it 
into the file header in each file (see the other files , like the register 
definitions or so) for details.

> The driver makes it possible to use a regular application UART as
> the U-Boot output console for MXS CPUs.
> 
> Signed-off-by: Andreas Wass <andreas.wass@dalelven.com>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
>  Changes for v2:
>    - Added comment that regs-uartapp.h is pulled from LTIB
>    - BM_ prefixes removed and _MASK suffixes added instead
>    - BP_ prefixes removed and _OFFSET suffixes added instead
>    - BF_ defines removed altogether
>    - CONFIG_MXS_AUART_CLK renamed to MXS_AUART_CLK and guarding ifndef
> removed - Added comments describing what is set and unset during init of
> driver - Added newline that was accidently removed from serial.c

Nitpicks follow.

>  arch/arm/include/asm/arch-mxs/regs-uartapp.h | 236
> +++++++++++++++++++++++++++ drivers/serial/Makefile                      |
>   1 +
>  drivers/serial/mxs_auart.c                   | 139 ++++++++++++++++
>  drivers/serial/serial.c                      |   2 +
>  4 files changed, 378 insertions(+)
>  create mode 100644 arch/arm/include/asm/arch-mxs/regs-uartapp.h
>  create mode 100644 drivers/serial/mxs_auart.c
> 
> diff --git a/arch/arm/include/asm/arch-mxs/regs-uartapp.h
> b/arch/arm/include/asm/arch-mxs/regs-uartapp.h new file mode 100644
> index 0000000..60a0812
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-mxs/regs-uartapp.h
> @@ -0,0 +1,236 @@
> +/*
> + * Freescale MXS UARTAPP Register Definitions
> + *
> + * Copyright (C) 2013 Andreas Wass <andreas.wass@dalelven.com>
> + *
> + * Based on code from LTIB:
> + * Copyright 2008-2010 Freescale Semiconductor, Inc. All Rights Reserved.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef __ARCH_ARM___MXS_UARTAPP_H
> +#define __ARCH_ARM___MXS_UARTAPP_H
> +
> +#include <asm/imx-common/regs-common.h>
> +
> +#ifndef __ASSEMBLY__
> +struct mxs_uartapp_regs {
> +	mxs_reg_32(hw_uartapp_ctrl0)
> +	mxs_reg_32(hw_uartapp_ctrl1)
> +	mxs_reg_32(hw_uartapp_ctrl2)
> +	mxs_reg_32(hw_uartapp_linectrl)
> +	mxs_reg_32(hw_uartapp_linectrl2)
> +	mxs_reg_32(hw_uartapp_intr)
> +	mxs_reg_32(hw_uartapp_data)
> +	mxs_reg_32(hw_uartapp_stat)
> +	mxs_reg_32(hw_uartapp_debug)
> +	mxs_reg_32(hw_uartapp_version)
> +	mxs_reg_32(hw_uartapp_autobaud)
> +};
> +#endif
> +
> +
> +#define UARTAPP_CTRL0_SFTRST_MASK				(1 << 31)
> +#define UARTAPP_CTRL0_CLKGATE_MASK			(1 << 30)
> +#define UARTAPP_CTRL0_RUN_MASK				(1 << 29)
> +#define UARTAPP_CTRL0_RX_SOURCE_MASK			(1 << 28)
> +#define UARTAPP_CTRL0_RXTO_ENABLE_MASK			(1 << 27)
> +#define UARTAPP_CTRL0_RXTIMEOUT_OFFSET			(1 << 4)
> +#define UARTAPP_CTRL0_RXTIMEOUT_MASK			0x07FF0000
> +#define UARTAPP_CTRL0_XFER_COUNT_OFFSET			0
> +#define UARTAPP_CTRL0_XFER_COUNT_MASK			0x0000FFFF
> +
> +#define UARTAPP_CTRL1_RSVD2_OFFSET				29
> +#define UARTAPP_CTRL1_RSVD2_MASK				0xE0000000
> +
> +#define UARTAPP_CTRL1_RUN_MASK				(1 << 28)
> +#define UARTAPP_CTRL1_RSVD1_OFFSET				16
> +#define UARTAPP_CTRL1_RSVD1_MASK				0x0FFF0000
> +
> +#define UARTAPP_CTRL1_XFER_COUNT_OFFSET			0
> +#define UARTAPP_CTRL1_XFER_COUNT_MASK			0x0000FFFF
> +
> +#define UARTAPP_CTRL2_INVERT_RTS_MASK			(1 << 31)
> +#define UARTAPP_CTRL2_INVERT_CTS_MASK			(1 << 30)
> +#define UARTAPP_CTRL2_INVERT_TX_MASK			(1 << 29)
> +#define UARTAPP_CTRL2_INVERT_RX_MASK			(1 << 28)
> +#define UARTAPP_CTRL2_RTS_SEMAPHORE_MASK			(1 << 27)
> +#define UARTAPP_CTRL2_DMAONERR_MASK			(1 << 26)
> +#define UARTAPP_CTRL2_TXDMAE_MASK				(1 << 25)
> +#define UARTAPP_CTRL2_RXDMAE_MASK				(1 << 24)
> +#define UARTAPP_CTRL2_RSVD2_MASK				(1 << 23)
> +#define UARTAPP_CTRL2_RXIFLSEL_OFFSET			20
> +#define UARTAPP_CTRL2_RXIFLSEL_MASK			0x00700000
> +
> +#define BV_UARTAPP_CTRL2_RXIFLSEL__NOT_EMPTY		0x0
> +#define BV_UARTAPP_CTRL2_RXIFLSEL__ONE_QUARTER		0x1
> +#define BV_UARTAPP_CTRL2_RXIFLSEL__ONE_HALF		0x2
> +#define BV_UARTAPP_CTRL2_RXIFLSEL__THREE_QUARTERS	0x3
> +#define BV_UARTAPP_CTRL2_RXIFLSEL__SEVEN_EIGHTHS	0x4
> +#define BV_UARTAPP_CTRL2_RXIFLSEL__INVALID5		0x5
> +#define BV_UARTAPP_CTRL2_RXIFLSEL__INVALID6		0x6
> +#define BV_UARTAPP_CTRL2_RXIFLSEL__INVALID7		0x7

BV ... this needs fixing. It's usually defined as (val << something)

[...]

> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> index 697f2bb..4c45bfa 100644
> --- a/drivers/serial/Makefile
> +++ b/drivers/serial/Makefile
> @@ -38,6 +38,7 @@ COBJS-$(CONFIG_SCIF_CONSOLE) += serial_sh.o
>  COBJS-$(CONFIG_ZYNQ_SERIAL) += serial_zynq.o
>  COBJS-$(CONFIG_BFIN_SERIAL) += serial_bfin.o
>  COBJS-$(CONFIG_FSL_LPUART) += serial_lpuart.o
> +COBJS-$(CONFIG_MXS_AUART) += mxs_auart.o
> 
>  ifndef CONFIG_SPL_BUILD
>  COBJS-$(CONFIG_USB_TTY) += usbtty.o
> diff --git a/drivers/serial/mxs_auart.c b/drivers/serial/mxs_auart.c
> new file mode 100644
> index 0000000..eaca6a4
> --- /dev/null
> +++ b/drivers/serial/mxs_auart.c
> @@ -0,0 +1,139 @@
> +/*
> + * Driver to use an application UART as console output for Freescale
> + * MXS devices.

Freescale i.MX23/i.MX28 AUART driver

> + *
> + * Copyright (C) 2013 Andreas Wass <andreas.wass@dalelven.com>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +#include <common.h>
> +#include <asm/io.h>
> +#include <serial.h>
> +#include <linux/compiler.h>
> +#include <asm/arch/regs-base.h>
> +#include <asm/arch/regs-uartapp.h>
> +#include <asm/arch/sys_proto.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#ifndef CONFIG_MXS_AUART_BASE
> +#error "CONFIG_MXS_AUART_BASE must be set to the base UART to use"
> +#endif
> +
> +/* AUART clock always supplied by XTAL and always 24MHz */
> +#define MXS_AUART_CLK 24000000
> +
> +static struct mxs_uartapp_regs *get_uartapp_registers(void)
> +{
> +	return (struct mxs_uartapp_regs*)CONFIG_MXS_AUART_BASE;
> +}
> +
> +/**
> + * Sets the baud rate and settings.
> + * The settings are: 8 data bits, no parit and 1 stop bit.
> + */
> +void mxs_auart_setbrg(void)
> +{
> +	u32 div;
> +	u32 linectrl = 0;
> +	struct mxs_uartapp_regs *regs = get_uartapp_registers();
> +
> +	/*
> +	 * From i.MX28 datasheet:
> +	 * div is calculated by calculating UARTCLK*32/baudrate, rounded to int
> +	 * div must be between 0xEC and 0x003FFFC0 inclusive
> +	 * Lowest 6 bits of div goes in BAUD_DIVFRAC part of LINECTRL register
> +	 * Next 16 bits goes in BAUD_DIVINT part of LINECTRL register
> +	 */
> +	div = (MXS_AUART_CLK * 32) / CONFIG_BAUDRATE;

You're not checking the div value here.

> +	linectrl |= ((div & 0x3F) << UARTAPP_LINECTRL_BAUD_DIVFRAC_OFFSET)
> +		& UARTAPP_LINECTRL_BAUD_DIVFRAC_MASK;
> +	linectrl |= ((div >> 6) << UARTAPP_LINECTRL_BAUD_DIVINT_OFFSET)
> +		& UARTAPP_LINECTRL_BAUD_DIVINT_MASK;
> +
> +	/* Word length: 8 bits */
> +	linectrl |= ((0x03) << UARTAPP_LINECTRL_WLEN_OFFSET)
> +		& UARTAPP_LINECTRL_WLEN_MASK;

You should clean these three magic numbers above.

> +	/* Enable FIFOs. */
> +	linectrl |= UARTAPP_LINECTRL_FEN_MASK;
> +
> +	/* Write above settings, no parity, 1 stop bit */
> +	writel(linectrl, &regs->hw_uartapp_linectrl);
> +}
> +
> +int mxs_auart_init(void)
> +{
> +	struct mxs_uartapp_regs *regs = get_uartapp_registers();
> +	/* Reset everything */
> +	mxs_reset_block(&regs->hw_uartapp_ctrl0_reg);
> +	/* Disable interrupts */
> +	writel(0, &regs->hw_uartapp_intr);
> +	/* Set baud rate and settings */
> +	serial_setbrg();
> +	/*
> +	 * Disable RTS and CTS,
> +	 * use values in LINECTRL register instead of LINECTRL2
> +	 */

Won't this fit on one line ?

[...]