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