Re: [Qemu-devel] [PATCH v5 02/11] ARM: exynos4210: UART support

[Peter Maydell <peter.maydell@linaro.org>]

On 23 December 2011 11:40, Evgeny Voevodin <e.voevodin@samsung.com> wrote:
> From: Maksim Kozlov <m.kozlov@samsung.com>
>
> Add basic support of exynos4210 UART
>
> Signed-off-by: Evgeny Voevodin <e.voevodin@samsung.com>
> ---
>  Makefile.target      |    2 +-
>  hw/exynos4210.c      |   24 ++
>  hw/exynos4210.h      |    9 +
>  hw/exynos4210_uart.c |  670 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 704 insertions(+), 1 deletions(-)
>  create mode 100644 hw/exynos4210_uart.c
>
> diff --git a/Makefile.target b/Makefile.target
> index fc308bf..0246947 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -336,7 +336,7 @@ obj-arm-y = integratorcp.o versatilepb.o arm_pic.o arm_timer.o
>  obj-arm-y += arm_boot.o pl011.o pl031.o pl050.o pl080.o pl110.o pl181.o pl190.o
>  obj-arm-y += versatile_pci.o
>  obj-arm-y += realview_gic.o realview.o arm_sysctl.o arm11mpcore.o a9mpcore.o
> -obj-arm-y += exynos4_boards.o exynos4210.o
> +obj-arm-y += exynos4_boards.o exynos4210.o exynos4210_uart.o
>  obj-arm-y += arm_mptimer.o
>  obj-arm-y += armv7m.o armv7m_nvic.o stellaris.o pl022.o stellaris_enet.o
>  obj-arm-y += pl061.o
> diff --git a/hw/exynos4210.c b/hw/exynos4210.c
> index 62d0ad5..afef4cb 100644
> --- a/hw/exynos4210.c
> +++ b/hw/exynos4210.c
> @@ -26,8 +26,19 @@
>  #include "arm-misc.h"
>  #include "exynos4210.h"
>
> +

stray blank line

> +
> +#ifdef DEBUG_UART
> +/* The only meaning of life - debugging. This functions should be only used
> + * inside PRINT_DEBUG_... macroses
> + */

"macros"
> +    default:
> +        level = 0;
> +        PRINT_ERROR("Wrong UART chennel number: %d\n", s->channel);

"channel"

> +
> +#ifdef DEBUG_IRQ
> +        fprintf(stderr, "UART%d: IRQ have been raised: %08x\n",
> +                s->channel, s->reg[I_(UINTP)]);

"has been"

> +    case UINTP:
> +        s->reg[I_(UINTP)]  &= ~val;
> +        /* XXX: It's the assumption that it works in this way */

Can you expand this comment a little to clarify exactly what the
assumption you're making is?

-- PMM