Re: [RFC 18/29] xen/arm: add generic UART to get the device in the device tree

[Julien Grall <julien.grall@linaro.org>]

On 04/29/2013 04:51 PM, Ian Campbell wrote:

> On Mon, 2013-04-29 at 00:02 +0100, Julien Grall wrote:
>> This generic UART will find the right UART via xen command line
>> with com1=myserial.
>>
>> "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@linaro.org>
>> ---
>>  xen/arch/arm/setup.c        |    3 +-
>>  xen/drivers/char/Makefile   |    1 +
>>  xen/drivers/char/arm-uart.c |   76 +++++++++++++++++++++++++++++++++++++++++++
>>  xen/include/xen/serial.h    |    8 +++++
>>  4 files changed, 87 insertions(+), 1 deletion(-)
>>  create mode 100644 xen/drivers/char/arm-uart.c
>>
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index 51f4bba..e5d8724 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -434,8 +434,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..e242ae2
>> --- /dev/null
>> +++ b/xen/drivers/char/arm-uart.c
>> @@ -0,0 +1,76 @@
>> +/*
>> + * xen/drivers/char/arm-uart.c
>> + *
>> + * Generic ARM uart retrieved via the device tree
>> + *
>> + * Julien Grall <julien.grall@linaro.org>
>> + * 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 serial port with string: devname
>> + * Where devname is the alias of the device in the device tree
>> + */
>> +static char __initdata opt_com1[30] = "";
>> +string_param("com1", opt_com1);
> 
> com1 is a bit of an x86-ism. Can we use e.g. uart0 or does common code
> constrain us here?

>

> Better would be to arrange things such that console=myserial does the
> lookup. Then myserial=<foo> sets options for that device.
> 
> I think you would need to add a call to dt_console_init to
> console_init_preirq(), in the "Where should console output go?" loop.


I don't see an easy solution with dt_console_init.
serial_parse_handle hardcodes the name of the serial, which only
supports com1, com2, dbgp. Sadly, the serial initialization is made in
the same function.

If I understand your suggestion, you suggest to use console=myserial
where *myserial* is the alias on the DTB, right? In this case the name
could clash with the hardcoded ones in Xen that is not good.

-- 
Julien