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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-04-29 at 18:24 +0100, Julien Grall wrote:
> 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@xxxxxxxxxx>
> >> ---
> >>  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@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 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.

Couldn't it also hardcode a call to a "is this a dt serial alias"
function?

> If I understand your suggestion, you suggest to use console=myserial
> where *myserial* is the alias on the DTB, right?

Right.

> In this case the name could clash with the hardcoded ones in Xen that is not 
> good.

console=dt:<alias> ? Or to be honest I'm not sure that having the dt
aliases shadow the Xen hardcoded ones would be a bad thing -- the x86
centric com<X> definition hardcoded in Xen is hardly likely to be useful
on ARM or any DT using platform.

Ian.


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