Re: [Qemu-devel] [RFC PATCH 4/6] arm/xlnx-zynq: use FDT names for the Cadence UART

[Igor Mammedov <imammedo@redhat.com>]

On Mon, 8 Jan 2018 10:51:38 -0300
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

> On 01/08/2018 10:17 AM, Thomas Huth wrote:
> > On 08.01.2018 13:54, Igor Mammedov wrote:  
> >> On Thu,  4 Jan 2018 11:40:44 -0300
> >> Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >>  
> >>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >>> ---
> >>>  hw/arm/xilinx_zynq.c | 1 +
> >>>  hw/arm/xlnx-zynqmp.c | 2 +-
> >>>  2 files changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> >>> index 1836a4ed45..c45c364583 100644
> >>> --- a/hw/arm/xilinx_zynq.c
> >>> +++ b/hw/arm/xilinx_zynq.c
> >>> @@ -236,6 +236,7 @@ static void zynq_init(MachineState *machine)
> >>>      sysbus_create_simple("xlnx,ps7-usb", 0xE0002000, pic[53-IRQ_OFFSET]);
> >>>      sysbus_create_simple("xlnx,ps7-usb", 0xE0003000, pic[76-IRQ_OFFSET]);
> >>>  
> >>> +    /* "xlnx,xuartps" */
> >>>      cadence_uart_create(0xE0000000, pic[59 - IRQ_OFFSET], serial_hds[0]);
> >>>      cadence_uart_create(0xE0001000, pic[82 - IRQ_OFFSET], serial_hds[1]);
> >>>  
> >>> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> >>> index 325642058b..38f038786c 100644
> >>> --- a/hw/arm/xlnx-zynqmp.c
> >>> +++ b/hw/arm/xlnx-zynqmp.c
> >>> @@ -155,7 +155,7 @@ static void xlnx_zynqmp_init(Object *obj)
> >>>      }
> >>>  
> >>>      for (i = 0; i < XLNX_ZYNQMP_NUM_UARTS; i++) {
> >>> -        object_initialize(&s->uart[i], sizeof(s->uart[i]), TYPE_CADENCE_UART);
> >>> +        object_initialize(&s->uart[i], sizeof(s->uart[i]), "xlnx,zynqmp-uart");  
> >>
> >> I don't think that commas are valid symbol in type names
> >> (if I recall correctly it should be letters, numbers and '-')  
> > 
> > At least commas are a real PITA when you try to use such devices with
> > the "-device" CLI parameter. We should try to avoid this if possible, I
> > think.  
> 
> Ok, good to know.
> 
> I was following Linux Device Tree names [1] to avoid handling some
> fdt_qemu_to_linux[] & fdt_linux_to_qemu conversion arrays.
> 
> What about keeping using the QEMU default name for -device CLI param and
> allow aliases for FDT parsing?
> With this series the cadence_uart_info is now:
> 
> static const TypeInfo cadence_uart_info = {
>     .name          = TYPE_CADENCE_UART,
>     .aliases       = (const char * []) {
>                         "cdns,uart-r1p8",
>                         "xlnx,xuartps",
>                         "cdns,uart-r1p12",
>                         "xlnx,zynqmp-uart",
>                         NULL
>                      },
No need to pollute Object for this,
It might be better if you make it interface and type will implement it.
But honestly putting desired name in place at the place FDT binding is created
is much simpler and one can adapt it to a specific board as needed.


> 
> [1]:
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/serial/cdns,uart.txt
>