On Sun, 5 May 2024 at 15:06, Inès Varhol <ines.var...@telecom-paris.fr> wrote:
>
> Signed-off-by: Inès Varhol <ines.var...@telecom-paris.fr>

In general you should try to avoid commits with no commit message.
Sometimes there really isn't anything to say beyond what the
subject line is, but that should be the exception rather than
the usual thing.

> ---
>  include/hw/misc/stm32l4x5_syscfg.h |  1 +
>  hw/arm/stm32l4x5_soc.c             |  2 ++
>  hw/misc/stm32l4x5_syscfg.c         | 26 ++++++++++++++++++++++++++
>  3 files changed, 29 insertions(+)
>
> diff --git a/include/hw/misc/stm32l4x5_syscfg.h 
> b/include/hw/misc/stm32l4x5_syscfg.h
> index 23bb564150..c450df2b9e 100644
> --- a/include/hw/misc/stm32l4x5_syscfg.h
> +++ b/include/hw/misc/stm32l4x5_syscfg.h
> @@ -48,6 +48,7 @@ struct Stm32l4x5SyscfgState {
>      uint32_t swpr2;
>
>      qemu_irq gpio_out[GPIO_NUM_PINS];
> +    Clock *clk;
>  };
>
>  #endif
> diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
> index 38f7a2d5d9..fb2afa6cfe 100644
> --- a/hw/arm/stm32l4x5_soc.c
> +++ b/hw/arm/stm32l4x5_soc.c
> @@ -236,6 +236,8 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc, 
> Error **errp)
>
>      /* System configuration controller */
>      busdev = SYS_BUS_DEVICE(&s->syscfg);
> +    qdev_connect_clock_in(DEVICE(&s->syscfg), "clk",
> +        qdev_get_clock_out(DEVICE(&(s->rcc)), "syscfg-out"));
>      if (!sysbus_realize(busdev, errp)) {
>          return;
>      }
> diff --git a/hw/misc/stm32l4x5_syscfg.c b/hw/misc/stm32l4x5_syscfg.c
> index a5a1ce2680..a82864c33d 100644
> --- a/hw/misc/stm32l4x5_syscfg.c
> +++ b/hw/misc/stm32l4x5_syscfg.c
> @@ -26,6 +26,10 @@
>  #include "trace.h"
>  #include "hw/irq.h"
>  #include "migration/vmstate.h"
> +#include "hw/clock.h"
> +#include "hw/qdev-clock.h"
> +#include "qapi/visitor.h"
> +#include "qapi/error.h"
>  #include "hw/misc/stm32l4x5_syscfg.h"
>  #include "hw/gpio/stm32l4x5_gpio.h"
>
> @@ -202,6 +206,14 @@ static void stm32l4x5_syscfg_write(void *opaque, hwaddr 
> addr,
>      }
>  }
>
> +static void clock_freq_get(Object *obj, Visitor *v,
> +    const char *name, void *opaque, Error **errp)
> +{
> +    Stm32l4x5SyscfgState *s = STM32L4X5_SYSCFG(obj);
> +    uint32_t clock_freq_hz = clock_get_hz(s->clk);
> +    visit_type_uint32(v, name, &clock_freq_hz, errp);
> +}
> +
>  static const MemoryRegionOps stm32l4x5_syscfg_ops = {
>      .read = stm32l4x5_syscfg_read,
>      .write = stm32l4x5_syscfg_write,
> @@ -225,6 +237,18 @@ static void stm32l4x5_syscfg_init(Object *obj)
>      qdev_init_gpio_in(DEVICE(obj), stm32l4x5_syscfg_set_irq,
>                        GPIO_NUM_PINS * NUM_GPIOS);
>      qdev_init_gpio_out(DEVICE(obj), s->gpio_out, GPIO_NUM_PINS);
> +    s->clk = qdev_init_clock_in(DEVICE(s), "clk", NULL, s, 0);
> +    object_property_add(obj, "clock-freq-hz", "uint32", clock_freq_get, NULL,
> +                        NULL, NULL);

Why do we need this property? The clock on this device is an input,
so the device doesn't control its frequency.

> +}
> +
> +static void stm32l4x5_syscfg_realize(DeviceState *dev, Error **errp)
> +{
> +    Stm32l4x5SyscfgState *s = STM32L4X5_SYSCFG(dev);
> +    if (!clock_has_source(s->clk)) {
> +        error_setg(errp, "SYSCFG: clk input must be connected");
> +        return;
> +    }
>  }
>
>  static const VMStateDescription vmstate_stm32l4x5_syscfg = {
> @@ -241,6 +265,7 @@ static const VMStateDescription vmstate_stm32l4x5_syscfg 
> = {
>          VMSTATE_UINT32(swpr, Stm32l4x5SyscfgState),
>          VMSTATE_UINT32(skr, Stm32l4x5SyscfgState),
>          VMSTATE_UINT32(swpr2, Stm32l4x5SyscfgState),
> +        VMSTATE_CLOCK(clk, Stm32l4x5SyscfgState),

Adding a field to the vmstate means we must bump the version number,
since it's a migration compatibility break.

>          VMSTATE_END_OF_LIST()
>      }
>  };

thanks
-- PMM

Reply via email to