[PATCH 1/4] hw/misc: Create STM32L4x5 SYSCFG clock
This commit creates a clock in STM32L4x5 SYSCFG and wires it up to the
corresponding clock from STM32L4x5 RCC.
A read-only QOM property allowing to read the clock frequency is added
(it will be used in a QTest).
Signed-off-by: Inès Varhol
---
Hello,
Several people noticed that replicating the code in the
different devices is a bad idea (cf cover letter).
One proposition is to directly add the clock property
in `qdev_init_clock_in()`.
Would that be acceptable and are there other alternatives
(allowing to the clock frequency from a Qtest)?
Best regards,
Inès Varhol
include/hw/misc/stm32l4x5_syscfg.h | 1 +
hw/arm/stm32l4x5_soc.c | 2 ++
hw/misc/stm32l4x5_syscfg.c | 30 --
3 files changed, 31 insertions(+), 2 deletions(-)
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(>syscfg);
+qdev_connect_clock_in(DEVICE(>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..7e6125383e 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, _freq_hz, errp);
+}
+
static const MemoryRegionOps stm32l4x5_syscfg_ops = {
.read = stm32l4x5_syscfg_read,
.write = stm32l4x5_syscfg_write,
@@ -225,12 +237,24 @@ 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);
+}
+
+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 = {
.name = TYPE_STM32L4X5_SYSCFG,
-.version_id = 1,
-.minimum_version_id = 1,
+.version_id = 2,
+.minimum_version_id = 2,
.fields = (VMStateField[]) {
VMSTATE_UINT32(memrmp, Stm32l4x5SyscfgState),
VMSTATE_UINT32(cfgr1, Stm32l4x5SyscfgState),
@@ -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),
VMSTATE_END_OF_LIST()
}
};
@@ -251,6 +276,7 @@ static void stm32l4x5_syscfg_class_init(ObjectClass *klass,
void *data)
ResettableClass *rc = RESETTABLE_CLASS(klass);
dc->vmsd = _stm32l4x5_syscfg;
+dc->realize = stm32l4x5_syscfg_realize;
rc->phases.hold = stm32l4x5_syscfg_hold_reset;
}
--
2.43.2
Re: [PATCH 1/4] hw/misc: Create STM32L4x5 SYSCFG clock
- Le 7 Mai 24, à 11:50, peter maydell peter.mayd...@linaro.org a écrit :
> On Sun, 5 May 2024 at 15:06, Inès Varhol wrote:
>>
>> Signed-off-by: Inès Varhol
>
> 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.
Hello,
Understood, I'll add messages.
>
>> ---
>> 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(>syscfg);
>> +qdev_connect_clock_in(DEVICE(>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, _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.
Using a QOM property allows to read the clock frequency from a QTest.
(npcm7xx_pwm-test.c does this, I didn't find other examples of reading a
frequency)
Best regards,
Inès Varhol
Re: [PATCH 1/4] hw/misc: Create STM32L4x5 SYSCFG clock
On Sun, 5 May 2024 at 15:06, Inès Varhol wrote:
>
> Signed-off-by: Inès Varhol
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(>syscfg);
> +qdev_connect_clock_in(DEVICE(>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, _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
[PATCH 1/4] hw/misc: Create STM32L4x5 SYSCFG clock
Signed-off-by: Inès Varhol
---
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(>syscfg);
+qdev_connect_clock_in(DEVICE(>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, _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);
+}
+
+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),
VMSTATE_END_OF_LIST()
}
};
@@ -251,6 +276,7 @@ static void stm32l4x5_syscfg_class_init(ObjectClass *klass,
void *data)
ResettableClass *rc = RESETTABLE_CLASS(klass);
dc->vmsd = _stm32l4x5_syscfg;
+dc->realize = stm32l4x5_syscfg_realize;
rc->phases.hold = stm32l4x5_syscfg_hold_reset;
}
--
2.43.2
