Re: [PATCH-for-5.1? 2/4] hw/arm/xilinx_zynq: Call qdev_connect_clock_in() before DeviceRealize

[Alistair Francis]

On Mon, Aug 3, 2020 at 3:57 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Clock canonical name is set in device_set_realized (see the block
> added to hw/core/qdev.c in commit 0e6934f264).
> If we connect a clock after the device is realized, this code is
> not executed. This is currently not a problem as this name is only
> used for trace events, however this disrupt tracing.
>
> Fix by calling qdev_connect_clock_in() before realizing.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/arm/xilinx_zynq.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index cf6d9757b5..969ef0727c 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -222,18 +222,18 @@ static void zynq_init(MachineState *machine)
>                            1, 0x0066, 0x0022, 0x0000, 0x0000, 0x0555, 0x2aa,
>                            0);
>
> -    /* Create slcr, keep a pointer to connect clocks */
> -    slcr = qdev_new("xilinx,zynq_slcr");
> -    sysbus_realize_and_unref(SYS_BUS_DEVICE(slcr), &error_fatal);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(slcr), 0, 0xF8000000);
> -
>      /* Create the main clock source, and feed slcr with it */
>      zynq_machine->ps_clk = CLOCK(object_new(TYPE_CLOCK));
>      object_property_add_child(OBJECT(zynq_machine), "ps_clk",
>                                OBJECT(zynq_machine->ps_clk));
>      object_unref(OBJECT(zynq_machine->ps_clk));
>      clock_set_hz(zynq_machine->ps_clk, PS_CLK_FREQUENCY);
> +
> +    /* Create slcr, keep a pointer to connect clocks */
> +    slcr = qdev_new("xilinx,zynq_slcr");
>      qdev_connect_clock_in(slcr, "ps_clk", zynq_machine->ps_clk);
> +    sysbus_realize_and_unref(SYS_BUS_DEVICE(slcr), &error_fatal);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(slcr), 0, 0xF8000000);
>
>      dev = qdev_new(TYPE_A9MPCORE_PRIV);
>      qdev_prop_set_uint32(dev, "num-cpu", 1);
> @@ -257,19 +257,19 @@ static void zynq_init(MachineState *machine)
>      dev = qdev_new(TYPE_CADENCE_UART);
>      busdev = SYS_BUS_DEVICE(dev);
>      qdev_prop_set_chr(dev, "chardev", serial_hd(0));
> +    qdev_connect_clock_in(dev, "refclk",
> +                          qdev_get_clock_out(slcr, "uart0_ref_clk"));
>      sysbus_realize_and_unref(busdev, &error_fatal);
>      sysbus_mmio_map(busdev, 0, 0xE0000000);
>      sysbus_connect_irq(busdev, 0, pic[59 - IRQ_OFFSET]);
> -    qdev_connect_clock_in(dev, "refclk",
> -                          qdev_get_clock_out(slcr, "uart0_ref_clk"));
>      dev = qdev_new(TYPE_CADENCE_UART);
>      busdev = SYS_BUS_DEVICE(dev);
>      qdev_prop_set_chr(dev, "chardev", serial_hd(1));
> +    qdev_connect_clock_in(dev, "refclk",
> +                          qdev_get_clock_out(slcr, "uart1_ref_clk"));
>      sysbus_realize_and_unref(busdev, &error_fatal);
>      sysbus_mmio_map(busdev, 0, 0xE0001000);
>      sysbus_connect_irq(busdev, 0, pic[82 - IRQ_OFFSET]);
> -    qdev_connect_clock_in(dev, "refclk",
> -                          qdev_get_clock_out(slcr, "uart1_ref_clk"));
>
>      sysbus_create_varargs("cadence_ttc", 0xF8001000,
>              pic[42-IRQ_OFFSET], pic[43-IRQ_OFFSET], pic[44-IRQ_OFFSET], 
> NULL);
> --
> 2.21.3
>
>