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

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

[Philippe Mathieu-Daudé]

This is not a critical issue, but still annoying if you use tracing.

Maybe it is worth fixing for forks that are based on release tags,
else consider it 5.2 material.

Anyway we can still discuss if qdev_connect_clock_in() is supposed to
be callable *after* DeviceRealize.

Philippe Mathieu-Daudé (4):
  hw/arm/xilinx_zynq: Uninline cadence_uart_create()
  hw/arm/xilinx_zynq: Call qdev_connect_clock_in() before DeviceRealize
  hw/qdev-clock: Uninline qdev_connect_clock_in()
  hw/qdev-clock: Avoid calling qdev_connect_clock_in after DeviceRealize

 include/hw/char/cadence_uart.h | 17 -----------------
 include/hw/qdev-clock.h        |  8 +++-----
 hw/arm/xilinx_zynq.c           | 24 +++++++++++++++++-------
 hw/core/qdev-clock.c           |  6 ++++++
 4 files changed, 26 insertions(+), 29 deletions(-)

-- 
2.21.3

[PATCH-for-5.1? 1/4] hw/arm/xilinx_zynq: Uninline cadence_uart_create()

[Philippe Mathieu-Daudé]

As we want to call qdev_connect_clock_in() before the device
is realized, we need to uninline cadence_uart_create() first.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/char/cadence_uart.h | 17 -----------------
 hw/arm/xilinx_zynq.c           | 14 ++++++++++++--
 2 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/include/hw/char/cadence_uart.h b/include/hw/char/cadence_uart.h
index ed7b58d31d..dabc49ea4f 100644
--- a/include/hw/char/cadence_uart.h
+++ b/include/hw/char/cadence_uart.h
@@ -53,21 +53,4 @@ typedef struct {
     Clock *refclk;
 } CadenceUARTState;
 
-static inline DeviceState *cadence_uart_create(hwaddr addr,
-                                        qemu_irq irq,
-                                        Chardev *chr)
-{
-    DeviceState *dev;
-    SysBusDevice *s;
-
-    dev = qdev_new(TYPE_CADENCE_UART);
-    s = SYS_BUS_DEVICE(dev);
-    qdev_prop_set_chr(dev, "chardev", chr);
-    sysbus_realize_and_unref(s, &error_fatal);
-    sysbus_mmio_map(s, 0, addr);
-    sysbus_connect_irq(s, 0, irq);
-
-    return dev;
-}
-
 #endif
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 32aa7323d9..cf6d9757b5 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -254,10 +254,20 @@ static void zynq_init(MachineState *machine)
     sysbus_create_simple(TYPE_CHIPIDEA, 0xE0002000, pic[53 - IRQ_OFFSET]);
     sysbus_create_simple(TYPE_CHIPIDEA, 0xE0003000, pic[76 - IRQ_OFFSET]);
 
-    dev = cadence_uart_create(0xE0000000, pic[59 - IRQ_OFFSET], serial_hd(0));
+    dev = qdev_new(TYPE_CADENCE_UART);
+    busdev = SYS_BUS_DEVICE(dev);
+    qdev_prop_set_chr(dev, "chardev", serial_hd(0));
+    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 = cadence_uart_create(0xE0001000, pic[82 - IRQ_OFFSET], serial_hd(1));
+    dev = qdev_new(TYPE_CADENCE_UART);
+    busdev = SYS_BUS_DEVICE(dev);
+    qdev_prop_set_chr(dev, "chardev", serial_hd(1));
+    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"));
 
-- 
2.21.3

Re: [PATCH-for-5.1? 1/4] hw/arm/xilinx_zynq: Uninline cadence_uart_create()

[Alistair Francis]

On Mon, Aug 3, 2020 at 3:57 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> As we want to call qdev_connect_clock_in() before the device
> is realized, we need to uninline cadence_uart_create() first.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

Alistair

> ---
>  include/hw/char/cadence_uart.h | 17 -----------------
>  hw/arm/xilinx_zynq.c           | 14 ++++++++++++--
>  2 files changed, 12 insertions(+), 19 deletions(-)
>
> diff --git a/include/hw/char/cadence_uart.h b/include/hw/char/cadence_uart.h
> index ed7b58d31d..dabc49ea4f 100644
> --- a/include/hw/char/cadence_uart.h
> +++ b/include/hw/char/cadence_uart.h
> @@ -53,21 +53,4 @@ typedef struct {
>      Clock *refclk;
>  } CadenceUARTState;
>
> -static inline DeviceState *cadence_uart_create(hwaddr addr,
> -                                        qemu_irq irq,
> -                                        Chardev *chr)
> -{
> -    DeviceState *dev;
> -    SysBusDevice *s;
> -
> -    dev = qdev_new(TYPE_CADENCE_UART);
> -    s = SYS_BUS_DEVICE(dev);
> -    qdev_prop_set_chr(dev, "chardev", chr);
> -    sysbus_realize_and_unref(s, &error_fatal);
> -    sysbus_mmio_map(s, 0, addr);
> -    sysbus_connect_irq(s, 0, irq);
> -
> -    return dev;
> -}
> -
>  #endif
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 32aa7323d9..cf6d9757b5 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -254,10 +254,20 @@ static void zynq_init(MachineState *machine)
>      sysbus_create_simple(TYPE_CHIPIDEA, 0xE0002000, pic[53 - IRQ_OFFSET]);
>      sysbus_create_simple(TYPE_CHIPIDEA, 0xE0003000, pic[76 - IRQ_OFFSET]);
>
> -    dev = cadence_uart_create(0xE0000000, pic[59 - IRQ_OFFSET], serial_hd(0));
> +    dev = qdev_new(TYPE_CADENCE_UART);
> +    busdev = SYS_BUS_DEVICE(dev);
> +    qdev_prop_set_chr(dev, "chardev", serial_hd(0));
> +    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 = cadence_uart_create(0xE0001000, pic[82 - IRQ_OFFSET], serial_hd(1));
> +    dev = qdev_new(TYPE_CADENCE_UART);
> +    busdev = SYS_BUS_DEVICE(dev);
> +    qdev_prop_set_chr(dev, "chardev", serial_hd(1));
> +    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"));
>
> --
> 2.21.3
>
>

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

[Philippe Mathieu-Daudé]

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>
---
 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

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
>
>

[PATCH-for-5.1? 3/4] hw/qdev-clock: Uninline qdev_connect_clock_in()

[Philippe Mathieu-Daudé]

We want to assert the device is not realized. To avoid overloading
this header including "hw/qdev-core.h", uninline the function first.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/qdev-clock.h | 6 +-----
 hw/core/qdev-clock.c    | 5 +++++
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/hw/qdev-clock.h b/include/hw/qdev-clock.h
index a340f65ff9..a897f7c9d0 100644
--- a/include/hw/qdev-clock.h
+++ b/include/hw/qdev-clock.h
@@ -71,11 +71,7 @@ Clock *qdev_get_clock_out(DeviceState *dev, const char *name);
  * Set the source clock of input clock @name of device @dev to @source.
  * @source period update will be propagated to @name clock.
  */
-static inline void qdev_connect_clock_in(DeviceState *dev, const char *name,
-                                         Clock *source)
-{
-    clock_set_source(qdev_get_clock_in(dev, name), source);
-}
+void qdev_connect_clock_in(DeviceState *dev, const char *name, Clock *source);
 
 /**
  * qdev_alias_clock:
diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
index 5cc1e82e51..f139b68b88 100644
--- a/hw/core/qdev-clock.c
+++ b/hw/core/qdev-clock.c
@@ -183,3 +183,8 @@ Clock *qdev_alias_clock(DeviceState *dev, const char *name,
 
     return ncl->clock;
 }
+
+void qdev_connect_clock_in(DeviceState *dev, const char *name, Clock *source)
+{
+    clock_set_source(qdev_get_clock_in(dev, name), source);
+}
-- 
2.21.3

Re: [PATCH-for-5.1? 3/4] hw/qdev-clock: Uninline qdev_connect_clock_in()

[Alistair Francis]

On Mon, Aug 3, 2020 at 3:57 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> We want to assert the device is not realized. To avoid overloading
> this header including "hw/qdev-core.h", uninline the function first.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

Alistair

> ---
>  include/hw/qdev-clock.h | 6 +-----
>  hw/core/qdev-clock.c    | 5 +++++
>  2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/qdev-clock.h b/include/hw/qdev-clock.h
> index a340f65ff9..a897f7c9d0 100644
> --- a/include/hw/qdev-clock.h
> +++ b/include/hw/qdev-clock.h
> @@ -71,11 +71,7 @@ Clock *qdev_get_clock_out(DeviceState *dev, const char *name);
>   * Set the source clock of input clock @name of device @dev to @source.
>   * @source period update will be propagated to @name clock.
>   */
> -static inline void qdev_connect_clock_in(DeviceState *dev, const char *name,
> -                                         Clock *source)
> -{
> -    clock_set_source(qdev_get_clock_in(dev, name), source);
> -}
> +void qdev_connect_clock_in(DeviceState *dev, const char *name, Clock *source);
>
>  /**
>   * qdev_alias_clock:
> diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
> index 5cc1e82e51..f139b68b88 100644
> --- a/hw/core/qdev-clock.c
> +++ b/hw/core/qdev-clock.c
> @@ -183,3 +183,8 @@ Clock *qdev_alias_clock(DeviceState *dev, const char *name,
>
>      return ncl->clock;
>  }
> +
> +void qdev_connect_clock_in(DeviceState *dev, const char *name, Clock *source)
> +{
> +    clock_set_source(qdev_get_clock_in(dev, name), source);
> +}
> --
> 2.21.3
>
>

[PATCH-for-5.1? 4/4] hw/qdev-clock: Avoid calling qdev_connect_clock_in after DeviceRealize

[Philippe Mathieu-Daudé]

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.

Add a comment to document qdev_connect_clock_in() must be called
before the device is realized, and assert this condition.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/qdev-clock.h | 2 ++
 hw/core/qdev-clock.c    | 1 +
 2 files changed, 3 insertions(+)

diff --git a/include/hw/qdev-clock.h b/include/hw/qdev-clock.h
index a897f7c9d0..64ca4d266f 100644
--- a/include/hw/qdev-clock.h
+++ b/include/hw/qdev-clock.h
@@ -70,6 +70,8 @@ Clock *qdev_get_clock_out(DeviceState *dev, const char *name);
  *
  * Set the source clock of input clock @name of device @dev to @source.
  * @source period update will be propagated to @name clock.
+ *
+ * Must be called before @dev is realized.
  */
 void qdev_connect_clock_in(DeviceState *dev, const char *name, Clock *source);
 
diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
index f139b68b88..47ecb5b4fa 100644
--- a/hw/core/qdev-clock.c
+++ b/hw/core/qdev-clock.c
@@ -186,5 +186,6 @@ Clock *qdev_alias_clock(DeviceState *dev, const char *name,
 
 void qdev_connect_clock_in(DeviceState *dev, const char *name, Clock *source)
 {
+    assert(!dev->realized);
     clock_set_source(qdev_get_clock_in(dev, name), source);
 }
-- 
2.21.3

Re: [PATCH-for-5.1? 4/4] hw/qdev-clock: Avoid calling qdev_connect_clock_in after DeviceRealize

[Alistair Francis]

On Mon, Aug 3, 2020 at 3:59 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.
>
> Add a comment to document qdev_connect_clock_in() must be called
> before the device is realized, and assert this condition.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

Alistair

> ---
>  include/hw/qdev-clock.h | 2 ++
>  hw/core/qdev-clock.c    | 1 +
>  2 files changed, 3 insertions(+)
>
> diff --git a/include/hw/qdev-clock.h b/include/hw/qdev-clock.h
> index a897f7c9d0..64ca4d266f 100644
> --- a/include/hw/qdev-clock.h
> +++ b/include/hw/qdev-clock.h
> @@ -70,6 +70,8 @@ Clock *qdev_get_clock_out(DeviceState *dev, const char *name);
>   *
>   * Set the source clock of input clock @name of device @dev to @source.
>   * @source period update will be propagated to @name clock.
> + *
> + * Must be called before @dev is realized.
>   */
>  void qdev_connect_clock_in(DeviceState *dev, const char *name, Clock *source);
>
> diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
> index f139b68b88..47ecb5b4fa 100644
> --- a/hw/core/qdev-clock.c
> +++ b/hw/core/qdev-clock.c
> @@ -186,5 +186,6 @@ Clock *qdev_alias_clock(DeviceState *dev, const char *name,
>
>  void qdev_connect_clock_in(DeviceState *dev, const char *name, Clock *source)
>  {
> +    assert(!dev->realized);
>      clock_set_source(qdev_get_clock_in(dev, name), source);
>  }
> --
> 2.21.3
>
>

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

[Philippe Mathieu-Daudé]

ping now that 5.2 is opened :)

On 8/3/20 12:56 PM, Philippe Mathieu-Daudé wrote:
> This is not a critical issue, but still annoying if you use tracing.
> 
> Maybe it is worth fixing for forks that are based on release tags,
> else consider it 5.2 material.
> 
> Anyway we can still discuss if qdev_connect_clock_in() is supposed to
> be callable *after* DeviceRealize.
> 
> Philippe Mathieu-Daudé (4):
>   hw/arm/xilinx_zynq: Uninline cadence_uart_create()
>   hw/arm/xilinx_zynq: Call qdev_connect_clock_in() before DeviceRealize
>   hw/qdev-clock: Uninline qdev_connect_clock_in()
>   hw/qdev-clock: Avoid calling qdev_connect_clock_in after DeviceRealize
> 
>  include/hw/char/cadence_uart.h | 17 -----------------
>  include/hw/qdev-clock.h        |  8 +++-----
>  hw/arm/xilinx_zynq.c           | 24 +++++++++++++++++-------
>  hw/core/qdev-clock.c           |  6 ++++++
>  4 files changed, 26 insertions(+), 29 deletions(-)
> 

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

[Peter Maydell]

On Mon, 3 Aug 2020 at 11:56, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> This is not a critical issue, but still annoying if you use tracing.
>
> Maybe it is worth fixing for forks that are based on release tags,
> else consider it 5.2 material.
>
> Anyway we can still discuss if qdev_connect_clock_in() is supposed to
> be callable *after* DeviceRealize.
>
> Philippe Mathieu-Daudé (4):
>   hw/arm/xilinx_zynq: Uninline cadence_uart_create()
>   hw/arm/xilinx_zynq: Call qdev_connect_clock_in() before DeviceRealize
>   hw/qdev-clock: Uninline qdev_connect_clock_in()
>   hw/qdev-clock: Avoid calling qdev_connect_clock_in after DeviceRealize
>


Applied to target-arm.next, thanks.

-- PMM