Re: [Xen-devel] [PATCH V2 20/33] xen/arm: Add generic UART to get the device in the device tree

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-05-08 at 03:33 +0100, Julien Grall wrote:
> This generic UART will find the right UART via xen command line
> with dtuart=myserial.

I suppose there way to determine sensible default, since it differs on
every platform?

> "myserial" is the alias of the UART in the device tree. Xen will retrieve
> the information via the device tree and call the initialization function for
> this specific UART thanks to the device API.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>

You need to CC code code maintainers when you change core code. Keir
added.
> 
> Changes in v2:
>     - Use dtuart parameter instead of com1. The first one is more arm
>     while the latter is more x86
> ---
>  xen/arch/arm/setup.c         |    3 +-
>  xen/drivers/char/Makefile    |    1 +
>  xen/drivers/char/arm-uart.c  |   81 
> ++++++++++++++++++++++++++++++++++++++++++
>  xen/drivers/char/serial.c    |    6 ++++
>  xen/include/asm-arm/config.h |    2 +-
>  xen/include/xen/serial.h     |    7 ++++
>  6 files changed, 98 insertions(+), 2 deletions(-)
>  create mode 100644 xen/drivers/char/arm-uart.c
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index e4228f7..7b2df8b 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -435,8 +435,9 @@ void __init start_xen(unsigned long boot_phys_offset,
>  #ifdef EARLY_UART_ADDRESS
>      /* TODO Need to get device tree or command line for UART address */
>      pl011_init(0, FIXMAP_ADDR(FIXMAP_CONSOLE));
> -    console_init_preirq();
>  #endif
> +    arm_uart_init();
> +    console_init_preirq();
>  
>      /* FIXME: Do something smarter */
>      dt_switch_to_printk();
> diff --git a/xen/drivers/char/Makefile b/xen/drivers/char/Makefile
> index ab2246d..e68a54a 100644
> --- a/xen/drivers/char/Makefile
> +++ b/xen/drivers/char/Makefile
> @@ -2,4 +2,5 @@ obj-y += console.o
>  obj-$(HAS_NS16550) += ns16550.o
>  obj-$(HAS_PL011) += pl011.o
>  obj-$(HAS_EHCI) += ehci-dbgp.o
> +obj-$(CONFIG_ARM) += arm-uart.o
>  obj-y += serial.o
> diff --git a/xen/drivers/char/arm-uart.c b/xen/drivers/char/arm-uart.c
> new file mode 100644
> index 0000000..c76875e
> --- /dev/null
> +++ b/xen/drivers/char/arm-uart.c
> @@ -0,0 +1,81 @@
> +/*
> + * xen/drivers/char/arm-uart.c
> + *
> + * Generic ARM uart retrieved via the device tree
> + *
> + * Julien Grall <julien.grall@xxxxxxxxxx>
> + * Copyright (c) 2013 Linaro Limited.
> + *
> + * 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.
> + */
> +
> +#include <asm/device.h>
> +#include <asm/early_printk.h>
> +#include <asm/types.h>
> +#include <xen/console.h>
> +#include <xen/device_tree.h>
> +#include <xen/mm.h>
> +#include <xen/serial.h>
> +
> +/*
> + * Configure UART port with a string:
> + * alias
> + *
> + * @alias: alias used in the device tree for the UART
> + * TODO: Implement config  in each UART driver.
> + */
> +static char __initdata opt_dtuart[30] = "";
> +string_param("dtuart", opt_dtuart);
> +
> +void __init arm_uart_init(void)

This (and the file and struct serial_arm_defaults etc) could perhaps be
better called dt_uart_init etc? There's nothing inherently ARM specific
here.

> +{
> +    struct dt_device_node *dev;
> +    int ret;
> +    u64 addr, size;
> +    struct serial_arm_defaults defaults;
> +    const char *devalias = opt_dtuart;
> +
> +    if ( !console_has("dtuart") || !strcmp(opt_dtuart, "") )
> +    {
> +        early_printk("No console\n");
> +        return;
> +    }
> +
> +    early_printk("Looking for UART console %s\n", devalias);
> +    dev = dt_find_node_by_alias(devalias);
> +
> +    if ( !dev )
> +    {
> +        early_printk("Unable to find device \"%s\"\n", devalias);
> +        return;
> +    }
> +
> +    /* TODO: Handle UART with 0 or multiple base address */
> +    ret = dt_device_get_address(dev, 0, &addr, &size);
> +    if ( ret )
> +    {
> +        early_printk("Unable to retrieve the base address of the serial\n");
> +        return;
> +    }
> +
> +    clear_fixmap(FIXMAP_CONSOLE);
> +    set_fixmap(FIXMAP_CONSOLE, addr >> PAGE_SHIFT, DEV_SHARED);
> +
> +    addr = FIXMAP_ADDR(FIXMAP_CONSOLE) + (addr & (PAGE_SIZE - 1));
> +
> +    defaults.index = 0;
> +    defaults.register_base_address = addr;
> +
> +    ret = device_init(dev, DEVICE_SERIAL, &defaults);
> +
> +    if ( ret )
> +        early_printk("Unable to initialize serial: %d\n", ret);
> +}
> diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c
> index 0ae7e4d..c4c4a84 100644
> --- a/xen/drivers/char/serial.c
> +++ b/xen/drivers/char/serial.c
> @@ -271,6 +271,12 @@ int __init serial_parse_handle(char *conf)
>          goto common;
>      }
>  
> +    if ( !strncmp(conf, "dtuart", 5) )
> +    {
> +        handle = SERHND_COM1;

Do you mean COM1 here? Or did you intend to add SERHND_DT?

> +        goto common;
> +    }
> +
>      if ( strncmp(conf, "com", 3) )
>          goto fail;
>  
> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index 98a3a43..8ed72f5 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -39,7 +39,7 @@
>  
>  #define CONFIG_VIDEO 1
>  
> -#define OPT_CONSOLE_STR "com1"
> +#define OPT_CONSOLE_STR "dtuart"
>  
>  #ifdef MAX_PHYS_CPUS
>  #define NR_CPUS MAX_PHYS_CPUS
> diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
> index 5de5171..f548f8b 100644
> --- a/xen/include/xen/serial.h
> +++ b/xen/include/xen/serial.h
> @@ -9,6 +9,7 @@
>  #ifndef __XEN_SERIAL_H__
>  #define __XEN_SERIAL_H__
>  
> +#include <xen/init.h>
>  #include <xen/spinlock.h>
>  
>  struct cpu_user_regs;
> @@ -156,6 +157,12 @@ void ns16550_init(int index, struct ns16550_defaults 
> *defaults);
>  void ehci_dbgp_init(void);
>  
>  void pl011_init(int index, unsigned long register_base_address);
> +/* Default value for UART on ARM boards */
> +struct serial_arm_defaults {
> +    int index;                              /* Serial index */
> +    unsigned long register_base_address;    /* Virtual base address */
> +};
> +void __init arm_uart_init(void);
>  
>  struct physdev_dbgp_op;
>  int dbgp_op(const struct physdev_dbgp_op *);



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel