Re: [Xen-devel] [PATCH v2 2/2] xen/arm: Add Marvell ARMADA 3700 early printk support

[Andre Przywara <andre.przywara@xxxxxxx>]

Hi,

On 03/04/18 14:49, Amit Singh Tomar wrote:
> Signed-off-by: Amit Singh Tomar <amittomer25@xxxxxxxxx>
> ---
> Changes since v1:
>     * Removed header file dependency. 
>             
> ---
>  docs/misc/arm/early-printk.txt     |  1 +
>  xen/arch/arm/Rules.mk              |  1 +
>  xen/arch/arm/arm64/debug-mvebu.inc | 50 
> ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 52 insertions(+)
>  create mode 100644 xen/arch/arm/arm64/debug-mvebu.inc
> 
> diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
> index 20a8af8..f765f59 100644
> --- a/docs/misc/arm/early-printk.txt
> +++ b/docs/misc/arm/early-printk.txt
> @@ -41,6 +41,7 @@ the name of the machine:
>    - juno: printk with pl011 on Juno platform
>    - lager: printk with SCIF0 on Renesas R-Car H2 processors
>    - midway: printk with the pl011 on Calxeda Midway processors
> +  - mvebu: printk with the MVEBU for Marvell Armada 3700 SoCs
>    - omap5432: printk with UART3 on TI OMAP5432 processors
>    - rcar3: printk with SCIF2 on Renesas R-Car Gen3 processors
>    - seattle: printk with pl011 for AMD Seattle processor
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index b66c19f..f264592 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -36,6 +36,7 @@ EARLY_PRINTK_hikey960       := pl011,0xfff32000
>  EARLY_PRINTK_juno           := pl011,0x7ff80000
>  EARLY_PRINTK_lager          := scif,0xe6e60000
>  EARLY_PRINTK_midway         := pl011,0xfff36000
> +EARLY_PRINTK_mvebu          := mvebu,0xd0012000
>  EARLY_PRINTK_omap5432       := 8250,0x48020000,2
>  EARLY_PRINTK_rcar3          := scif,0xe6e88000
>  EARLY_PRINTK_seattle        := pl011,0xe1010000
> diff --git a/xen/arch/arm/arm64/debug-mvebu.inc 
> b/xen/arch/arm/arm64/debug-mvebu.inc
> new file mode 100644
> index 0000000..ac48889
> --- /dev/null
> +++ b/xen/arch/arm/arm64/debug-mvebu.inc
> @@ -0,0 +1,50 @@
> +/*
> + * xen/drivers/char/mvebu3700-uart.c
> + *
> + * Driver for Marvell MVEBU UART.
> + *
> + * Copyright (c) 2018, Amit Singh Tomar <amittomer25@xxxxxxxxx>.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms and conditions of the GNU General Public
> + * License, version 2, as published by the Free Software Foundation.
> + *
> + * 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, see 
> <http://www.gnu.org/licenses/>.
> + */
> +
> +#define UART_STATUS_REG     0xc 

There is a trailing space in this line. Also you could use 0x0c instead
to match the line below.

> +#define UART_TX_REG         0x04
> +
> +/*
> + * MVEBU UART wait UART to be ready to transmit
> + * xb: register which contains the UART base address
> + * c: scratch register
> + */
> +.macro early_uart_ready xb c
> +1:
> +        ldrh   w\c, [\xb, #UART_STATUS_REG]  /* <- status register */
> +        tst    w\c, #(1 << 13)                        /* <- Check TXFIFO EMP 
> bit */

I don't think it's necessary to check for the FIFO being empty. Do as
the Linux driver does: Check for !full. So use bit 11 here and negate
the branch condition below. Will test this later tonight.

And while you are on it: Can you please align the comments, also make
sure they don't break the line (for instance just remove the " <- ")?

Cheers,
Andre.

> +        beq    1b                            /* <- Wait for the UART to be 
> ready */
> +.endm
> +
> +/*
> + * MVEBU UART transmit character
> + * xb: register which contains the UART base address
> + * wt: register which contains the character to transmit
> + */
> +.macro early_uart_transmit xb wt
> +     strb  \wt, [\xb, #UART_TX_REG]
> +.endm
> +
> +/*
> + * Local variables:
> + * mode: ASM
> + * indent-tabs-mode: nil
> + * End:
> + */
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel